saadtajwar commented on PR #23617: URL: https://github.com/apache/datafusion/pull/23617#issuecomment-5004034699
> Thanks for updating this, I think my earlier comment may have pushed this a little too far in the helper direction š > > What I was trying to get at was mostly reducing the repeated range-partitioning setup, not adding a bunch of extra test scaffolding / coverage. I think we can keep this much tighter: > > * one positive projection-swap test that verifies the range key is rewritten when its projected column index changes > * one negative projection-swap test that covers dropping a single key and dropping part of a compound key > * two sort-pushdown tests: supported when the range repartition maintains input order, unsupported when it does not > * keep the repartitioned() None test > > Iād drop the extra execution sortedness test since this PR really about the planning behavior. > > For helpers I think the useful ones are only the small range-specific setup things like build range partitioning from cols, build a projection from cols, unwrap/assert range partitioning. Haha no worries, my bad - updated! Thanks again for reviewing :) -- 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]
