-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
destination-redshift: Fix StackOverflowError with eager rendering of nested jooq function call sql. #33877
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
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.
don't forget to update the arrayConcatStmt javadoc to say it's not recursive :P
.../java/io/airbyte/integrations/destination/redshift/typing_deduping/RedshiftSqlGenerator.java
Outdated
Show resolved
Hide resolved
...a/io/airbyte/integrations/destination/redshift/typing_deduping/RedshiftSqlGeneratorTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Gireesh Sreepathi <[email protected]>
Signed-off-by: Gireesh Sreepathi <[email protected]>
Signed-off-by: Gireesh Sreepathi <[email protected]>
Signed-off-by: Gireesh Sreepathi <[email protected]>
Signed-off-by: Gireesh Sreepathi <[email protected]>
6117010
to
4a9b2b3
Compare
…nested jooq function call sql. (airbytehq#33877) Signed-off-by: Gireesh Sreepathi <[email protected]>
…nested jooq function call sql. (airbytehq#33877) Signed-off-by: Gireesh Sreepathi <[email protected]>
What
Fixes #33759
How
Building the recursive
function()
doesn't seem to be the problem, the StackOverflowError happens at 2000 columns, which is arbitrary depending on the Stack Size allocated by JVM (which can be increased by-Xms
). The problem seems to be lurking in how the SQL string rendering happens wherefunction(...function(...
nested references are creating StackOverflowError. Fix is to render the SQL at every step of nesting to avoid deep stack.