srielau commented on PR #59315:
URL: https://github.com/apache/spark/pull/59315#issuecomment-6100376632
Thanks for the careful read. I reverted the three
`excludeFiltersOnCharColumns` call sites locally and reran the SPARK-60102
tests.
- `BasicCharVarcharTestSuite` `SPARK-60102: CSV CHAR filters are not pushed
down` still passed. `CAST('ab' AS CHAR(4))` is folded to the padded literal
before `DataSourceStrategy.translateLeafNodeFilter`, so the pushed `EqualTo`
already matches the padded parsed value. There is no SQL-path row loss on
master for this query.
- `c = 'ab'` is even less of a parser mismatch: analysis compares `cast(c AS
STRING) = 'ab'`, which is not translated as `EqualTo(c, ...)`. Only
`IsNotNull(c)` is pushed, and the post-scan predicate still rejects `'ab '`.
- `UnivocityParserSuite` `SPARK-60102: CHAR column filters are not applied
in the parser` failed without the exclusion (`None` vs `Some([ab ,1])`). That
is the hand-built `EqualTo("c", "ab")` case.
So the analysis is right: the end-to-end test does not guard a SQL-path bug,
and Spark's own `c = 'ab'` would also reject the padded value after the scan.
The exclusion only changes parser-level skipping for source Filters that
already carry an unpadded CHAR literal.
I wrapped the temp view as suggested. Happy to drop the CHAR-column
exclusion (and keep only the parser unit test, or close the PR) if that cost is
not worth a synthetic mismatch.
--
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]
