-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Postgres: column name truncation spike #36805
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
General thumbs up, just the toStreamConfig
method is getting a little unruly and we should break it up into sub methods
@@ -161,6 +161,45 @@ constructor( | |||
var i = 1 |
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.
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.
done in c610197?w=1. also cleaned up some of the kotlin autoconvert cruft.
// but this interface + our superclass are weirdly complicated, so plausibly something is missing | ||
@Override | ||
public String getIdentifier(final String name) { | ||
return truncate(super.getIdentifier(name)); |
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 already made another mess here unify that with call to getIdentifier
?
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.
done in 8291ba5.
3ca93b5
to
5bbae1f
Compare
c1204a2
to
f29d835
Compare
7ddd15b
to
f2bf8e0
Compare
8291ba5
to
74dde57
Compare
/publish-java-cdk
|
stacked on #36620.
See also #36808 for "just throw an error".we end up with e.g.
ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd
(63 chars long) andddddddddddddddddddddddddddddd461dddddddddddddddddddddd_rirqs2
(note the461
in the middle), when the original column names ared<510>d_rirqs1
andd<510>d_rirqs2
. Not convinced this is actually great behavior.also, fix the catalogparser tests to have more stringent asserts. Previously they were actually just completely useless, because we were mocking the wrong method. The asserts should now catch this problem, since we assert on the actual table/column names.
submitted https://github.com/airbytehq/airbyte-internal-issues/issues/6989 to make collision logic better in general.