LiaCastaneda commented on code in PR #20416:
URL: https://github.com/apache/datafusion/pull/20416#discussion_r2851892219
##########
datafusion/physical-expr/src/expressions/dynamic_filters.rs:
##########
@@ -327,6 +468,14 @@ impl DynamicFilterPhysicalExpr {
Arc::strong_count(self) > 1 || Arc::strong_count(&self.inner) > 1
}
+ /// Returns a unique identifier for the inner shared state.
+ ///
+ /// Useful for checking if two [`Arc<PhysicalExpr>`] with the same
+ /// underlying [`DynamicFilterPhysicalExpr`] are the same.
+ pub fn inner_id(&self) -> u64 {
+ Arc::as_ptr(&self.inner) as *const () as u64
Review Comment:
> The is_used() method returns bool if there is a strong count greater than
one in any of their inner or outer arcs, but you might perfectly have a strong
count greater than one just because of how you happen to lay out your Rust
code, and the filter might still be unused (e.g., in optimization passes, the
strong count can be 6 or 7 easily, and the filter is still unused)
I agree it’s a bit hacky and can be tricky to work with (for example, what
we saw in https://github.com/apache/datafusion/issues/19715). However, the
DynamicFilter isn’t currently public in any of the producer nodes, so it
shouldn’t be easy to clone it unintentionally. The only place it can be
obtained externally is via the `handle_child_pushdown_...` API on the consumer
side, which would imply the filter is actually being used. I might be missing
something, but in what situations would optimization passes increase the Arc
strong count without the filter actually being used?
Overall, though I agree we should look for a better long term solution for
this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
