kakiuwang-ui commented on issue #14540:
URL: https://github.com/apache/datafusion/issues/14540#issuecomment-5664345168
Ran @Jefffrey's reproduction on released 55.1.0 and traced what the fight
actually costs now, since the picture has changed since the original report.
The good news is that the plan no longer fails to converge. The optimizer
loop hashes a `LogicalPlanSignature` after every pass and breaks when it sees
one it has already recorded, so this query stops after three passes with a
stable, correct plan — the end of pass 2 and the end of pass 3 are
byte-identical.
What remains is a cycle *inside* a pass. In every pass after the first,
exactly three rules keep reporting a change and between them return the plan to
where it started:
```
Projection: __datafusion_extracted_1 AS device, ...
Filter: __datafusion_extracted_1 IS NOT NULL AND ...
Projection: __datafusion_extracted_1, __datafusion_extracted_2,
t1.timestamp
Filter: t1.date = Date32("2025-01-03") <-- (A)
Projection: get_field(t1.ids, 'id1') AS __datafusion_extracted_1,
... <-- (B)
TableScan: t1
```
- `push_down_filter` swaps (A) below (B), so the date filter runs before
`get_field` is evaluated.
- `push_down_leaf_projections` swaps (B) back below (A), so the extraction
runs for every row again.
- `optimize_projections` cleans up the projection the swap left behind,
landing back on the plan above.
So the two rules want the opposite relative order of the same adjacent pair,
and the one that runs later in the rule list wins. The net cost is one wasted
full optimizer pass for any query of this shape: without the fight the
signature check would fire one pass earlier.
Which of the two should yield is a plan-quality call I do not think I should
make unilaterally. For this query `push_down_filter`'s order looks better — the
date predicate does not reference the extracted columns, so filtering first
means `get_field` is evaluated on fewer rows — but `push_down_leaf_projections`
exists precisely to get leaf expressions close enough to the scan to be pushed
into it, so preferring the filter unconditionally may cost more than it saves
elsewhere.
Is there a preferred direction here? Two that seem plausible: teach
`push_down_filter` not to push a filter below a leaf-extraction projection it
will lose to anyway, or teach `push_down_leaf_projections` to stop at a filter
that does not reference the leaves it is extracting. Happy to implement
whichever you would rather have.
--
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]
