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

Breaking change since DF 44: swap_hash_join removed #14251

Closed
andygrove opened this issue Jan 23, 2025 · 3 comments
Closed

Breaking change since DF 44: swap_hash_join removed #14251

andygrove opened this issue Jan 23, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@andygrove
Copy link
Member

Describe the bug

Comet uses the public swap_hash_join function:
https://docs.rs/datafusion/latest/datafusion/physical_optimizer/join_selection/fn.swap_hash_join.html

This seems to have been removed (internally, there is a swap_inputs function that perhaps provides the same functionality but is not public as far as I can tell).

Our policy is to deprecate functions first rather than remove them, so I am wondering if we can reinstate this?

To Reproduce

No response

Expected behavior

No response

Additional context

No response

@andygrove
Copy link
Member Author

This isn't blocking Comet. I found that I just needed to make this change:

-                    let swapped_hash_join =
-                        swap_hash_join(hash_join.as_ref(), PartitionMode::Partitioned)?;
+                    let swapped_hash_join = hash_join.as_ref().swap_inputs(PartitionMode::Partitioned)?;

Deprecating the function with documentation explaining what the replacement is would be helpful though.

@buraksenn
Copy link
Contributor

I don't understand it is flagged as deprecated but is not deleted.

/// This function swaps the inputs of the given join operator.
/// This function is public so other downstream projects can use it
/// to construct `HashJoinExec` with right side as the build side.
#[deprecated(since = "45.0.0", note = "use HashJoinExec::swap_inputs instead")]
pub fn swap_hash_join(
hash_join: &HashJoinExec,
partition_mode: PartitionMode,
) -> Result<Arc<dyn ExecutionPlan>> {
hash_join.swap_inputs(partition_mode)
}

AFAIS, according to the documentation it will be deleted in version 50.0.0 or in 6 months

Deprecated methods will remain in the codebase for a period of 6 major versions or 6 months, whichever is longer, to provide users ample time to transition away from them.

Maybe I'm missing something but this is what I can see

@andygrove
Copy link
Member Author

Thanks @buraksenn, you are correct. I thought I was getting a compilation issue earlier, but it appears not. I will close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants