Skip to content

Commit ac24b01

Browse files
committed
Add manual Debug impl that omits current_cache to keep roundtrip tests stable
`datafusion-proto` roundtrip tests compare `format!("{:?}", expr)` on freshly-deserialized vs post-`current()`-called filters. Deriving `Debug` for `DynamicFilterPhysicalExpr` prints the cache slot, which differs across the assertion because one side has populated the cache and the other hasn't. The cache is a pure optimization artifact — exclude it from Debug output so equality by Debug format is preserved. Fixes `test_dynamic_filter_plan_roundtrip_dedupe` and `test_custom_node_with_dynamic_filter_dedup_roundtrip` regressions observed on the "cargo test hash collisions (amd64)" CI job.
1 parent 8041f43 commit ac24b01

1 file changed

Lines changed: 17 additions & 1 deletion

File tree

  • datafusion/physical-expr/src/expressions/dynamic_filters

datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ impl FilterState {
6666
/// For more background, please also see the [Dynamic Filters: Passing Information Between Operators During Execution for 25x Faster Queries blog]
6767
///
6868
/// [Dynamic Filters: Passing Information Between Operators During Execution for 25x Faster Queries blog]: https://datafusion.apache.org/blog/2025/09/10/dynamic-filters
69-
#[derive(Debug)]
7069
pub struct DynamicFilterPhysicalExpr {
7170
/// The original children of this PhysicalExpr, if any.
7271
/// This is necessary because the dynamic filter may be initialized with a placeholder (e.g. `lit(true)`)
@@ -97,6 +96,23 @@ pub struct DynamicFilterPhysicalExpr {
9796
nullable: Arc<RwLock<Option<bool>>>,
9897
}
9998

99+
impl std::fmt::Debug for DynamicFilterPhysicalExpr {
100+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
101+
// Manual impl deliberately omits `current_cache`: it is a pure
102+
// optimization artifact whose contents depend on whether
103+
// `current()` has been called, and roundtrip tests (e.g. in
104+
// `datafusion-proto`) compare `format!("{:?}", ..)` output.
105+
f.debug_struct("DynamicFilterPhysicalExpr")
106+
.field("children", &self.children)
107+
.field("remapped_children", &self.remapped_children)
108+
.field("inner", &self.inner)
109+
.field("state_watch", &self.state_watch)
110+
.field("data_type", &self.data_type)
111+
.field("nullable", &self.nullable)
112+
.finish()
113+
}
114+
}
115+
100116
/// Atomic internal state of a [`DynamicFilterPhysicalExpr`].
101117
///
102118
/// `expression_id` lives here because it identifies the actual filter expression `expr`.

0 commit comments

Comments
 (0)