zhuqi-lucas commented on PR #23599:
URL: https://github.com/apache/datafusion/pull/23599#issuecomment-6076931945
Thanks @2010YOUY01 — agreed on both issues, and the circular-dependency
point is fair.
The middle ground I'd suggest is splitting what this PR bundled:
1. **Declining a shape the rule doesn't understand** —
`filter.fetch().is_some()` → `return None`. No new behaviour, no new path, one
`is_some()` check.
2. **Handling a shape the rule doesn't see** — re-applying the embedded
projection. Real logic, and no default-order query reaches it, so no e2e test
can keep it honest.
I've cut the PR down to 1 (one commit, +45/-0). #21596 stays open for 2 —
worth noting its premise ("this happens when `ProjectionPushdown` runs before
`WindowTopN`") isn't true of the default list, which is exactly your point;
I'll note that on the issue so the next attempt starts from the right
assumption.
On testing: agreed a custom pipeline running one query is weaker than slt. I
think your idea already has a landing spot — `# configMatrix:` (#24493) re-runs
a whole slt file across a config sweep, and 5 files use it today. What it
sweeps is `ConfigOptions`, not rule lists, so the missing piece is just an
option selecting a pipeline variant (`datafusion.optimizer.pipeline = default |
reoptimize`); `prefer_hash_join` is precedent for an option that changes the
physical plan. Then `# configMatrix:
datafusion.optimizer.pipeline=default,reoptimize` gets the rest for free.
One caveat worth recording: the existing matrix files carry almost no
`EXPLAIN` (`sort_merge_join_matrix.slt` has one), since a different rule list
produces different plans and slt has no per-configuration expected output. So a
sweep proves *results agree* under the alternative pipeline, and "what extra
capability does it buy" needs its own plan assertions — your two asks fall out
as separate pieces.
Filed as #26152.
--
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]
