Reconcile schema nullability in BeamCoGBKJoinRel - #38868
Conversation
|
R: @Abacn |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a schema mismatch issue encountered during outer joins involving nested row structures. By reconciling the Calcite-derived schema with the actual underlying data schema, the changes ensure that nested field nullability is respected while maintaining correct top-level nullability for outer join operations. This prevents unnecessary type-guard exceptions during the execution of Beam SQL joins. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the join logic in BeamCoGBKJoinRel to reconcile Calcite's derived output schema with the actual data types, preventing type-equality guard failures caused by incorrect nullability reporting on nested struct fields. Additionally, a new test case is added to verify left outer joins with nested rows. Feedback suggests adding a defensive check to ensure the field count of the Calcite schema matches the size of the data fields before iterating, preventing a potential IndexOutOfBoundsException.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| Schema.Builder reconciled = Schema.builder(); | ||
| for (int i = 0; i < calciteSchema.getFieldCount(); i++) { |
There was a problem hiding this comment.
To prevent an unexpected IndexOutOfBoundsException during the loop, it is safer to defensively check that the field count of calciteSchema matches the size of dataFields before iterating.
| Schema.Builder reconciled = Schema.builder(); | |
| for (int i = 0; i < calciteSchema.getFieldCount(); i++) { | |
| if (calciteSchema.getFieldCount() != dataFields.size()) { | |
| throw new IllegalStateException( | |
| String.format( | |
| "Field count mismatch: Calcite schema has %d fields, but data schema has %d fields", | |
| calciteSchema.getFieldCount(), dataFields.size())); | |
| } | |
| Schema.Builder reconciled = Schema.builder(); | |
| for (int i = 0; i < calciteSchema.getFieldCount(); i++) { |
| } | ||
|
|
||
| @Test | ||
| public void testLeftOuterJoinWithNestedRows() { |
There was a problem hiding this comment.
I ran the test reverting BeamCoGBKJoinRel.java change it still passes
There was a problem hiding this comment.
Yeah, you're right - I think we don't actually need this backport after all, thanks
73ee0de to
62e46f0
Compare
2 participants
Addresses a schema mismatch bug that occurs during outer joins on schemas with nested rows (structs).
The Problem
When performing a or outer join, Calcite's type derivation automatically marks all fields from the nullable side of the join as nullable. For nested rows, Calcite recursively marks all nested fields inside the struct as nullable as well, even if they are defined as in the actual data schema.
When this Calcite-derived schema is passed to Beam's physical transform, it compares it against the actual data schema. Because the actual data still has nested fields while the Calcite schema expects them to be nullable, it trips a schema compatibility guard and throws an exception.
The Fix
Modified to perform schema reconciliation before outputting:
Added to to verify the fix.