jayshrivastava commented on issue #21207: URL: https://github.com/apache/datafusion/issues/21207#issuecomment-4298852366
Let me know what you think :) CI is green on my PR and I reviewed it carefully. Maybe it's easier to comment there. I dedupe using `PhysicalExpr::with_new_children` as well. Some differences: - I have `DynamicFilterSnapshot` so we can snapshot the `current()` expr and `generation()` atomically. In yours, we lock/unlock the mutex twice during serialization, so there's some risk there. It's a small risk, but a relevant nonetheless. - I preserve backwards compatability (ex. using the session id to salt the IDs, and deduping non-`DynamicFilterPhysicalExpr`). If we don't do this, we'd need to delete a bunch of tests from `roundtrip_physical_plan.rs` -- 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]
