[SPARK-60102][SQL] Skip CSV filter pushdown for CHAR columns

[srielau]

What changes were proposed in this pull request?

When CSV uses a first-class CHAR schema, do not push filters that reference a top-level CHAR column into UnivocityParser. Filters on other columns in the same schema continue to push down.

The exclusion is applied consistently in the V1 CSV file format, the V2 CSV scan builder, and UnivocityParser. VARCHAR filters are unchanged because VARCHAR values are not padded.

CSV value conversion remains unchanged: CHAR/VARCHAR semantics continue to run inside the STRING converter, matching from_json values. This PR does not introduce post-parse assignment or change PERMISSIVE/FAILFAST behavior.

JIRA: https://issues.apache.org/jira/browse/SPARK-60102

Related: #59325

Why are the changes needed?

The CSV converter pads a parsed CHAR(n) value to length n. A translated source filter can contain the corresponding unpadded literal and is compiled as STRING equality. Evaluating that filter inside the parser can therefore reject a row that satisfies the CHAR predicate after the scan.

For example, a CSV row containing ab under schema c CHAR(4) must satisfy:

SELECT * FROM csv_char WHERE c = CAST('ab' AS CHAR(4));

Skipping only filters that reference c preserves correctness while retaining pushdown for independent non-CHAR columns.

Does this PR introduce any user-facing change?

Yes, relative to current unreleased master, when first-class CHAR/VARCHAR semantics are enabled. CSV queries with a pushed filter on a CHAR column no longer lose matching rows. CSV parsing, CHAR padding, VARCHAR length checks, parse modes, and filters on non-CHAR columns are unchanged.

How was this patch tested?

Added coverage that:

• a CHAR-typed predicate returns the padded CSV row;
• a direct parser filter on CHAR is not applied;
• an independent INT filter on the same schema is still applied;
• a mixed CHAR and INT query returns the correct row.

build/sbt -Dsbt.override.build.repos=true \
  'catalyst/testOnly org.apache.spark.sql.catalyst.csv.UnivocityParserSuite -- -z SPARK-60102' \
  'sql/testOnly org.apache.spark.sql.BasicCharVarcharTestSuite -- -z SPARK-60102'

./dev/lint-scala also passed.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Cursor

[HyukjinKwon]

Review summary

The CSV change is small and is applied consistently in the V1 reader, the V2 scan builder, and the parser. Every excluded filter is still evaluated after the scan, so the change cannot drop rows.

The main open point is whether the problem it fixes can actually happen. Reading the source, CAST('ab' AS CHAR(4)) is folded to the padded literal before filter translation, so the pushed filter already seems to match the padded value, and the end-to-end test may pass without the change. I've raised this as a question in the decision challenge below rather than as a defect.

There is also one code-level nit: the new end-to-end test registers a temp view that is never dropped from the shared session.

Findings

1 total: 0 P0, 0 P1, 0 P2, 1 P3.

Nit (P3)

• New CSV CHAR test leaks the csv_char temp view — sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:2906 — see inline.

Decision challenges

Does the CSV CHAR filter mismatch reproduce, and does the e2e test guard it?

Could you confirm that the row loss this PR fixes actually reproduces on master? Reading the pinned source, I couldn't find a path where the pushed filter and the post-scan predicate disagree:

• DataSourceStrategy.translateLeafNodeFilter only translates column = Literal and copies the literal value unchanged (DataSourceStrategy.scala:640-643). So CAST('ab' AS CHAR(4)) must already be folded by ConstantFolding by the time a source Filter exists.
• Evaluating that cast pads to 'ab ' on both CHAR branches of ToStringBase.castToString (charTypeCast and charTypeWriteSideCheck).
• With a first-class c CHAR(4) schema, the attribute carries no raw-type metadata, so ApplyCharTypePadding adds no read-side projection or literal rewrite.
• OrderedFilters then evaluates EqualTo("c", "ab ") against the same padded 'ab ' that the post-scan filter sees.

If that is right, the new end-to-end assertions pass with or without the exclusion, and the change only gives up parser-level row skipping for every filter on a CHAR column. The only mismatch the tests demonstrate is the hand-built EqualTo("c", "ab") in UnivocityParserSuite, and Spark's own c = 'ab' would also reject 'ab ' after the scan.

I didn't build Spark or run the tests, so I may be missing an analyzer or optimizer step. Could you check whether SPARK-60102: CSV CHAR filters are not pushed down fails with the three excludeFiltersOnCharColumns calls reverted, or point to a query that loses rows on master? If such a query exists, using it in the end-to-end test would make the test guard the fix.

Generated by Omnigent on Databricks.

[srielau]

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.

[srielau]

Closing this PR.

from_csv CHAR/VARCHAR standard support is already complete on master: the STRING converter applies applyTextParseSemantics (pad CHAR, EXCEED_LIMIT_LENGTH on VARCHAR overflow). CSV does not parse MAP/ARRAY/STRUCT, so the JSON/XML map-key work in #59325 and #59316 does not apply here and does not remove any helper this path depends on.

The remaining CHAR-column filter skip does not fix a Spark-SQL-generated mismatch. Spark's own equality is either already padded (CAST(... AS CHAR(n))) or is STRING comparison after the scan (c = 'ab'). The exclusion only changes a hand-built EqualTo in the parser unit test, at the cost of skipping parser-level filters on every CHAR column. That is not worth landing.