Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(deps)!: Bump github.com/ClickHouse/clickhouse-go from v1.5.4 to v2.18.0 #14877

Closed
wants to merge 3 commits into from

Conversation

srebhan
Copy link
Member

@srebhan srebhan commented Feb 22, 2024

Summary

Version 1 of the ClickHouse SQL driver is no longer actively maintained. Therefore update to the new version 2.

Note: The DSN of v1 and v2 differs slightly, you might need to adapt your settings!

Checklist

  • No AI generated code was used in this PR

Related issues

@telegraf-tiger telegraf-tiger bot added the chore label Feb 22, 2024
@srebhan srebhan added plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins dependencies Pull requests that update a dependency file area/sql labels Feb 22, 2024
@telegraf-tiger
Copy link
Contributor

Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

⚠️ This pull request increases the Telegraf binary size by 1.05 % for linux amd64 (new size: 231.2 MB, nightly size 228.8 MB)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB RPM TAR GZ ZIP
amd64.deb aarch64.rpm darwin_amd64.tar.gz windows_amd64.zip
arm64.deb armel.rpm darwin_arm64.tar.gz windows_arm64.zip
armel.deb armv6hl.rpm freebsd_amd64.tar.gz windows_i386.zip
armhf.deb i386.rpm freebsd_armv7.tar.gz
i386.deb ppc64le.rpm freebsd_i386.tar.gz
mips.deb riscv64.rpm linux_amd64.tar.gz
mipsel.deb s390x.rpm linux_arm64.tar.gz
ppc64el.deb x86_64.rpm linux_armel.tar.gz
riscv64.deb linux_armhf.tar.gz
s390x.deb linux_i386.tar.gz
linux_mips.tar.gz
linux_mipsel.tar.gz
linux_ppc64le.tar.gz
linux_riscv64.tar.gz
linux_s390x.tar.gz

// exec sequence on query later.
if s.driverName == "clickhouse" {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this work wtih the v1 library?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so as the test did not complain. We can work on doing it (we already do have a special handling on the sql output plugin). The biggest issue is that Query() and Exec() will return two completely different data-types and we would need to implement this for only ClickHouse... Would you prefer we are doing this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you prefer we are doing this?

I'm not sure. From a let's not break anything that already works standpoint, then yes, but that seems quite involved. Are any of the other clickhouse libraries support this better out of the box?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is in the SQL driver. See upstream bug ClickHouse/clickhouse-go#1203. I suggest we are waiting for this one to get some answers...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I agree on holding.

@powersj powersj removed their assignment Feb 26, 2024
@powersj powersj marked this pull request as draft February 26, 2024 14:56
@powersj powersj closed this Feb 26, 2024
@powersj
Copy link
Contributor

powersj commented Feb 26, 2024

We will close this for now until we get a better idea of how we could migrate to v2 or some other library in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sql chore dependencies Pull requests that update a dependency file plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants