Remove unstable public methods for DynamicFilterPhysicalExpr after proto migration

[jayshrivastava]

Which issue does this PR close?

• Closes Remove unstable public methods for DynamicFilterPhysicalExpr after proto migration #23422
• Closes Port DynamicFilterPhysicalExpr to use try_to_proto / try_from_proto #22434

Rationale for this change

See #22418. After migrating DynamicFilterPhysicalExpr serialization from relying on leaking public internals, we can now remove these unstable methods because they aren't used outside the package.

pub struct Inner {
...
}

/// Return the filter's original children (before any remapping).
///
/// **Warning:** intended only for `datafusion-proto` (de)serialization.
/// Not a stable API.
pub fn original_children(&self) -> &[Arc<dyn PhysicalExpr>] {
    &self.children
}

/// Return the filter's remapped children, if any have been set via
/// [`PhysicalExpr::with_new_children`].
///
/// **Warning:** intended only for `datafusion-proto` (de)serialization.
/// Not a stable API.
pub fn remapped_children(&self) -> Option<&[Arc<dyn PhysicalExpr>]> {
    self.remapped_children.as_deref()
}

What changes are included in this PR?

See above.

Are these changes tested?

• Existing unit tests are migrated to private accessors
• There's no usage of these methods out side of datafusion/physical-expr/src/expressions/dynamic_filters

Are there any user-facing changes?

Unstable public methods and public types for DynamicFilterPhysicalExpr are private now.

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning apache/main
    Building datafusion-physical-expr v54.0.0 (current)
       Built [  34.396s] (current)
     Parsing datafusion-physical-expr v54.0.0 (current)
      Parsed [   0.048s] (current)
    Building datafusion-physical-expr v54.0.0 (baseline)
       Built [  26.908s] (baseline)
     Parsing datafusion-physical-expr v54.0.0 (baseline)
      Parsed [   0.049s] (baseline)
    Checking datafusion-physical-expr v54.0.0 -> v54.0.0 (no change; assume patch)
     Checked [   0.537s] 223 checks: 221 pass, 2 fail, 0 warn, 30 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/inherent_method_missing.ron

Failed in:
  DynamicFilterPhysicalExpr::original_children, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/d99ecf66a84cf963b65f867c2b81f3711ceba778/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:400
  DynamicFilterPhysicalExpr::remapped_children, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/d99ecf66a84cf963b65f867c2b81f3711ceba778/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:409
  DynamicFilterPhysicalExpr::from_parts, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/d99ecf66a84cf963b65f867c2b81f3711ceba778/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:418
  DynamicFilterPhysicalExpr::inner, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/d99ecf66a84cf963b65f867c2b81f3711ceba778/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:448

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/struct_missing.ron

Failed in:
  struct datafusion_physical_expr::expressions::DynamicFilterInner, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/d99ecf66a84cf963b65f867c2b81f3711ceba778/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:96

     Summary semver requires new major version: 2 major and 0 minor checks failed
    Finished [  62.957s] datafusion-physical-expr

[jayshrivastava]

Note we still have public methods like this

/// Get the dynamic filter expression for testing purposes.
/// Returns the dynamic filter expression for this hash join, if set.
pub fn dynamic_filter_expr(&self) -> Option<&Arc<DynamicFilterPhysicalExpr>> {}

/// Set the dynamic filter on this hash join.
///
/// Resets any internal state that depends on any existing dynamic filter.
///
/// Validates that the filter's children reference valid columns in
/// the probe (right) side's schema.
pub fn with_dynamic_filter_expr(
    mut self,
    filter: Arc<DynamicFilterPhysicalExpr>,
) -> Result<Self> {}

on dynamic filter producers like HashJoinExec.

I don't want to commit to removing those right now. They might come in handy for distributed datafusion since apply_expressions was reverted. I'm still thinking about the right APIs to have in datafusion for distibuted datafusion to access dynamic filters.

[jayshrivastava]

cc @adriangb

[adriangb]

Thanks! Since we did our best to mark these as not stable and they're relatively new I think we can make an exception to the API health policy. If this breaks things for anyone we'll apologize and add them back as deprecated. Seems worth the tradeoff.

[jayshrivastava]

Since we did our best to mark these as not stable and they're relatively new I think we can make an exception to the API health policy. If this breaks things for anyone we'll apologize and add them back as deprecated. Seems worth the tradeoff.

+1!

TYFR!