dongjoon-hyun commented on PR #58851:
URL: https://github.com/apache/spark/pull/58851#issuecomment-5704255866
Thank you for the fix, @pan3793. The direction looks right to me. I'd like
to raise two user-facing points that may be worth documenting in the PR
description (or covering with a test).
1. **Queries that currently succeed may fail, depending on conjunct
evaluation order.**
For example, `WHERE dep <> 'hr' AND to_int(dep) = 1`. Without pushdown,
`FilterExec` evaluates the translated conjunct first (`prioritizeFilters`) and
short-circuits, so `to_int` is never called on `'hr'` and the query succeeds.
With pushdown, `dep <> 'hr'` (first pass) and `to_int(dep) = 1` (second-pass
`PartitionPredicate`) are evaluated independently inside the connector, in
whatever order the connector chooses. Before this PR, fail-open happened to
give the correct answer here. After it, the query may fail, depending on the
connector. Since Spark doesn't guarantee evaluation order, this is not a
correctness issue, but the set of queries that "start failing" is wider than
the description currently suggests.
2. **The error can now surface earlier, and on empty partitions.**
As the new test notes, the error is raised while the scan is built. A
connector that evaluates partition predicates during scan building or planning
will therefore make `EXPLAIN SELECT ...` fail with a data error. Also, the
non-pushdown path fails only when a row in the partition is actually evaluated,
whereas partition-metadata evaluation can fail even for a partition with no
rows. Both seem acceptable, but it would be good to mention them in the
user-facing change section.
--
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]
