mustafasrepo commented on PR #6501: URL: https://github.com/apache/arrow-datafusion/pull/6501#issuecomment-1574428990
> I think this PR looks great -- thank you @mustafasrepo and adds a neat feature. cc @mingmwang in case you have any interest in reviewing this > > > However, because PhysicalSortExpr doesn't implement Hash trait (there is no trivial way to support this trait if any). We changed the EquivalentClass implementation so that it doesn't require Hash trait anymore. > > We hit something similar when trying to make `LogicalPlan` implement hash (because of the `LogicalPlan::Extension` variant that has a `Arc<dyn UserDefinedLogicalNode>` > > The solution we came up with was > > https://docs.rs/datafusion-expr/25.0.0/datafusion_expr/logical_plan/trait.UserDefinedLogicalNode.html#tymethod.dyn_hash > > And then implemented it like this: https://docs.rs/datafusion-expr/25.0.0/src/datafusion_expr/logical_plan/extension.rs.html#235-285 I will experiment with using `dyn hash`, I think this will simplify the structure. -- 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]
