Skip to content
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

Enable SortMergeJoin LeftAnti filtered fuzz tests #11535

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Conversation

comphead
Copy link
Contributor

Which issue does this PR close?

Closes #10872.

Rationale for this change

Fixed the fuzz test, previously the filter was not set up correctly

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the core Core DataFusion crate label Jul 18, 2024
@@ -217,6 +225,7 @@ async fn test_semi_join_1k() {

#[tokio::test]
async fn test_semi_join_1k_filtered() {
// NLJ vs HJ gives wrong result
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The HashJoin and NestedLoop Join gives different results for filtered joins fuzz tests.
In the same time HashJoin and SortMergeJoin results are correct.

Need to triage what is going on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Could you add a reference to the issue with the comment here

#[tokio::test]
async fn test_anti_join_1k_filtered() {
// NLJ vs HJ gives wrong result
Copy link
Member

Choose a reason for hiding this comment

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

Can you file an issue for this and reference it here?

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @comphead

@comphead
Copy link
Contributor Author

Thanks everyone for get this reviewed

@comphead comphead merged commit be130b4 into apache:main Jul 18, 2024
23 checks passed
@alamb
Copy link
Contributor

alamb commented Jul 19, 2024

FYI I have seen this test fail now a few times on unrelated PRs: #11555

Lordworms pushed a commit to Lordworms/arrow-datafusion that referenced this pull request Jul 23, 2024
* Enable LeftAnti filtered fuzz tests

* Enable LeftAnti filtered fuzz tests. Add git reference
wiedld pushed a commit to influxdata/arrow-datafusion that referenced this pull request Jul 31, 2024
* Enable LeftAnti filtered fuzz tests

* Enable LeftAnti filtered fuzz tests. Add git reference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SMJ LeftAnti filtered join fuzz test cannot pass
4 participants