rdettai commented on issue #1228: URL: https://github.com/apache/arrow-datafusion/issues/1228#issuecomment-969147085
Thanks for your work @xudong963 ! Sorry for getting into the conversation so late but I was in vacations these past two weeks 😉 A few remarks and questions: - why do we need to do all these changes to implement `PartialEq` (and `PartialOrd`) here? Why don't we just add an `fn eq(&self, other: & dyn TableProvider) -> bool` method on `TableProvider` and a then implement manually the `PartialEq` trait on the logical plan with a `match` statement? The method will surely be a bit verbose, but it would avoid refactoring the entire codebase. - even if we go for the solution where we want to implement `PartialEq` using `#[derive()]`, why do we need to extract all variants of `LogicalPlan` into structs? isn't it enough to do it for the variants with trait object fields? - finally if we really believe that it is worth extracting all the variants into structs and we do it in multiple issues, can we please have an umbrella issue with subtasks that link to all these steps? I might have missed some parts of the discussion, so please excuse me if my questions are a bit off 😄 -- 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]
