berkaysynnada commented on code in PR #14207:
URL: https://github.com/apache/datafusion/pull/14207#discussion_r1971311766
##########
datafusion/physical-optimizer/src/enforce_distribution.rs:
##########
@@ -987,16 +1003,22 @@ fn add_spm_on_top(input: DistributionContext) ->
DistributionContext {
fn remove_dist_changing_operators(
mut distribution_context: DistributionContext,
) -> Result<DistributionContext> {
+ let mut fetch = None;
while is_repartition(&distribution_context.plan)
|| is_coalesce_partitions(&distribution_context.plan)
|| is_sort_preserving_merge(&distribution_context.plan)
{
+ if is_sort_preserving_merge(&distribution_context.plan) {
Review Comment:
> I'm not sure if it'll sacrifice plan simplicity, but correctness is the
base.
There is a correctness issue we have now ?(unless you change the physical
optimizer list) -- Of course, we always aim having idempotent and orthogonal
rules, but I think to provide those, we shouldn't break the existing planning
results under the default and optimal rule set.
--
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]
