LiaCastaneda commented on code in PR #20416:
URL: https://github.com/apache/datafusion/pull/20416#discussion_r2829446739
##########
datafusion/physical-expr/src/expressions/dynamic_filters.rs:
##########
@@ -327,6 +467,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:
you might be interested in looking at
https://github.com/apache/datafusion/issues/19715#issuecomment-3733169909 we
found out that for some cases the `inner` struct was not the one being cloned
but the outer struct (because we never called `reassign_expr_columns ->
new_with_children`. I have an example DataSource repro
[here](https://github.com/LiaCastaneda/datafusion/commit/5dedd792670a5c0ea4be43e4ee3c88f355c5fc48)
were the inner struct is not clonned. I still haven't looked in what context
`inner_id` is used yet (I will continue reviewing tomorrow) but I wanted to
mention it in case it helps.
--
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]
