nseekhao commented on PR #7612: URL: https://github.com/apache/arrow-datafusion/pull/7612#issuecomment-1734540179
> Thank you @nseekhao and @Dandandan > > I think this code could be made simpler and left some suggestions about how to do so. > > That being said the only thing I think is needed prior to merge is some tests (so we don't break this code in some future refactoring). I don't think the simplifications are necessary I don't mind simplifying the code so I'll work on it following your earlier suggestions. As for the tests, I agree. Since, existing cases already test for mixed join condition ([roundtrip_non_equi_join()](https://github.com/apache/arrow-datafusion/blob/main/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs#L387)) and non-equi-only join ([roundtrip_non_equi_inner_join()](https://github.com/apache/arrow-datafusion/blob/main/datafusion/substrait/tests/cases/roundtrip_logical_plan.rs#L381)), are you thinking a new set of tests that specifically test for the absence of `post_join_filter`? Or did you have something else in mind? Thanks! -- 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]
