zhuqi-lucas commented on PR #21976:
URL: https://github.com/apache/datafusion/pull/21976#issuecomment-4436562586
Thanks for spelling that out @alamb — that's much cleaner than what I had.
Closing #22117 and switching to the in-place approach here. Plan:
1. Delete the disconnected `EnforceDistribution` / `EnforceSorting` modules
(they're not registered in the pipeline anymore — just dead code at this point).
2. Update the existing tests in
`core/tests/physical_optimizer/{enforce_distribution,enforce_sorting,enforce_sorting_monotonicity,replace_with_order_preserving_variants}.rs`
to call `EnsureRequirements::new()`. Keeps the diff focused on the rule swap,
exactly as you said.
3. Drop `ensure_requirements/new_tests.rs` and the inline tests in
`ensure_requirements/mod.rs` that duplicate the existing coverage.
One question on #3: a handful of cases in `new_tests.rs` cover behavior
that's genuinely new — idempotent re-application of pushdown_sorts, a couple of
multi-rule integration cases that didn't exist before. Worth keeping those
(either where they are or folded into the relevant existing files), or do you
want everything dropped and the new behavior covered later in a follow-up?
--
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]
