Skip to content

Conversation

@JkSelf
Copy link
Collaborator

@JkSelf JkSelf commented Apr 18, 2025

left    right
null    null
null    null
3       3

When performing a right join operation using above data, if we directly set rightRowIndex_ to 0 here, it will result in all final outcomes being empty in left side. This is because during the comparison process, the first value of the right input is null, leading to a comparison result of -1 consistently. Consequently, all the left-side data will be fully traversed, and the input will be set to null columns, resulting in the final output returning only the right-side columns, with all left-side columns set to null. And this PR addresses the above issue by skipping the null row index prior to comparing the results.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 18, 2025
@netlify
Copy link

netlify bot commented Apr 18, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 30b7150
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/68074dc85d31ac0008c6d0aa

@JkSelf JkSelf force-pushed the right-join-fix branch 2 times, most recently from db58ade to ad3d4f2 Compare April 22, 2025 05:13
@JkSelf
Copy link
Collaborator Author

JkSelf commented Apr 22, 2025

@xiaoxmeng Resolved all your comments. Please help to review again. Thanks.

Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@JkSelf thanks for the update!

@facebook-github-bot
Copy link
Contributor

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@kevinwilfong kevinwilfong left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@facebook-github-bot
Copy link
Contributor

@xiaoxmeng merged this pull request in 8285db4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants