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

Remove force_hash_collisions flag above the function dev_ceil #11701

Conversation

Kev1n8
Copy link
Contributor

@Kev1n8 Kev1n8 commented Jul 29, 2024

Which issue does this PR close?

Closes #11658.

Rationale for this change

The issue is due to a test helper function dev_ceil decorated by flag not force_hash_collision resulting it invisible for some test functions.

What changes are included in this PR?

I removed the #[cfg(not(force_hash_collisions))] above the function dev_ceil, which makes it visible for the test functions.

Are these changes tested?

Passed cargo test -p datafusion-physical-plan --features=force_hash_collisions

Are there any user-facing changes?

@Kev1n8 Kev1n8 marked this pull request as draft July 29, 2024 05:57
@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Aug 1, 2024

The tests are failing in

     #[template]
     #[rstest]
     fn batch_sizes(#[values(8192, 10, 5, 2, 1)] batch_size: usize) {}

I've singled out join_inner_two for evaluation as follows:

    #[tokio::test]
    async fn my_join_inner_two_test() -> Result<()> {
        let batch_size= 5;  // specifying batch_size to 5
        ...
    }

and I found out that the test could pass with test --package datafusion-physical-plan --lib joins::hash_join --features=force_hash_collisions.
However, it fails test --lib --features=force_hash_collisions,avro hash_join.
I've put effort in it but unfortunately fail to see what is causing this.

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Aug 1, 2024

I would like to close this draft. Sorry for not being able to complete it despite the effort I put in.

@alamb
Copy link
Contributor

alamb commented Aug 1, 2024

Thank you @Kev1n8 -- this one sounds tricky

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants