Skip to content

Conversation

@JkSelf
Copy link
Collaborator

@JkSelf JkSelf commented Dec 6, 2024

When running the TPCH Q21 query, we found that performing a left semi join followed by a left anti join in the same stage resulted in incorrect results. Upon investigation, we discovered that MergeSource was losing data in such complex join scenarios. This PR addresses the issue by placing the data from MergeSource into a queue, ensuring that the next method call only ends when the queue is empty.

@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 Dec 6, 2024
@netlify
Copy link

netlify bot commented Dec 6, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit ca404b7
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/675296352b8f8600082e18e4

@JkSelf
Copy link
Collaborator Author

JkSelf commented Dec 6, 2024

@pedroerp Can you help to review this PR? Thanks.

@netlify
Copy link

netlify bot commented Apr 18, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit c6a79dc
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6806f951a8a3140008371e6b

struct State {
bool atEnd = false;
RowVectorPtr data;
std::queue<RowVectorPtr> dataQueue;
Copy link
Contributor

Choose a reason for hiding this comment

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

in which case, we have more than one output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When executing a 1GB TPC-H Query 21 using multiple executors and multiple cores, the issue will arise.

@JkSelf
Copy link
Collaborator Author

JkSelf commented Jun 19, 2025

Closing this PR because the current code does not have any data loss issues

@JkSelf JkSelf closed this Jun 19, 2025
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. merge-join

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants