-
Notifications
You must be signed in to change notification settings - Fork 0
Remove workaround for _-prefixed column names
#57
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
base: main
Are you sure you want to change the base?
Conversation
|
Should we perhaps maintain this and check for CrateDB version? I assume that this will break people trying to use it with CrateDB < 6.2 |
| In order to learn more about Singer, Meltano, and friends, navigate to the | ||
| [Singer Intro](./docs/singer-intro.md). | ||
|
|
||
| Operating the package successfully needs CrateDB 6.2 or higher. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @surister. Thanks. Let me loop your comment into a thread.
Should we perhaps maintain this and check for CrateDB version? I assume that this will break people trying to use it with CrateDB < 6.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it will not work with lower versions of CrateDB. As I see the CrateDB 6.2 release might be far away (expecting 6.1 first?), this patch is probably not ready for releasing, and should be made a draft again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about pre-flight checks. I haven't seen many (if any) such measures in other vendor's adapters. In a perfect world, this adapter will vanish completely anyway if we could submit all specialities into the upstream target-postgres.
However, I see your point, and, as long as we maintain a separate adapter, and, most importantly, as long as it will not have a negative performance impact in high-traffic or high-volume environments, I will not object adding conveniency features like pre-flight version checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch includes a check for the CrateDB version, as suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NB: The package is in alpha/workbench mode, currently bearing just 46 downloads per month. In this spirit, we can easily produce (merge & release) this improvement, to make it a fully functional and significantly enhanced package for all users of CrateDB nightly and future CrateDB 6.2 users, even if this will only be released in Jan/Feb 2026.
All people who are currently using the package can easily stick with the currently released version. There is not much need to introduce or address backward compatibility issues.
About
With CrateDB 6.2, the workaround against
_-prefixed column names is no longer needed. Thanks! 🌻References
InvalidColumnNameException["_foo" conflicts with system column pattern]crate/crate#15161_-prefixed column names crate/dlt-cratedb#30_-prefixed column names crate/cratedb-fivetran-destination#83/cc @hammerhead, @surister, @kneth