[SPARK-59001][SQL] Empty2Null, text, and Hive prune fallback for CHAR/VARCHAR

[srielau]

What changes were proposed in this pull request?

SPARK-59001 (parent SPARK-58794).

Follow-up on first-class CHAR/VARCHAR (spark.sql.charVarchar.standardSemantics.enabled) so a few string-family call sites treat CharType / VarcharType like STRING.

• V1Writes applies Empty2Null to nullable CHAR/VARCHAR partition columns (dataType.isInstanceOf[StringType]). CHAR(n>0) still pads '' to spaces, so the empty CHAR case is CHAR(0).
• The V2 text data source accepts CHAR/VARCHAR as a string-family type.
• Hive metastore filter conversion still refuses CHAR/VARCHAR partition keys (varcharKeys in SupportedAttribute: Hive's trailing-blank comparison is not Spark's). When a predicate mentions such a key, prunePartitionsFastFallback prunes client-side with Spark's own predicates (CHAR compared without PAD SPACE; the test literal is 'a ' for CHAR(5)). Other empty-filter and MetaException fallbacks still honor metastorePartitionPruningFastFallback.

Why are the changes needed?

With first-class types, CHAR/VARCHAR stay in the plan instead of being rewritten to annotated STRING. Equality against StringType then skips them:

• empty partition values are not converted to NULL, so they become a distinct partition directory instead of __HIVE_DEFAULT_PARTITION__
• USING text rejects a CHAR/VARCHAR schema
• Hive partition filters on CHAR keys fetch every partition

The annotation-skipping idea was tried and dropped: ApplyCharTypePadding uses __CHAR_VARCHAR_TYPE_STRING as an idempotence marker, so the annotation is load-bearing.

Does this PR introduce any user-facing change?

Yes, only when spark.sql.charVarchar.standardSemantics.enabled is true (still off by default).

• Empty VARCHAR / CHAR(0) partition values become NULL like STRING.
• spark.read.schema("value CHAR(n)").text(...) is accepted.
• Hive CHAR partition filters prune to the matching partitions (client-side), and CHAR(5) = 'a' does not match a stored 'a ' unless the literal carries the pad or an RTRIM collation is used.

How was this patch tested?

• sql/testOnly *CharVarcharTestSuite *V1WriteCommandSuite: 164 succeeded
• hive/testOnly *HiveCharVarcharTestSuite: 58 succeeded
• hive/testOnly *HivePartitionFilteringSuite*: 360 succeeded (Hive 2.3-4.1)

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

Generated-by: Cursor Grok 4.6

[srielau]

SQL/storage review of the CHAR/VARCHAR follow-up (Empty2Null, Text V2, Hive prune).

The three production placements are right: isInstanceOf[StringType] matches how V1 text already admits the string family; leave SupportedAttribute refusing CHAR/VARCHAR so Hive does not apply PAD SPACE; client-side prune is the right fallback because metastorePartitionPruningFastFallback defaults to false.

Please address:

1. Scope the Hive fast-fallback override to predicates that mention CHAR/VARCHAR partition keys (not the whole partition schema), and keep the MetaException path on the existing conf.
2. Fix the Hive test: honest name, VARCHAR, and p = 'a' vs the padded literal.
3. Move or drop the SPARK-58801 parquet catalog/inference block; it does not exercise this PR.
4. Align the commit headline and new test names with SPARK-59001.

Nits: isInstanceOf[StringType] is not gated on standard semantics (preserve-only and collated STRING also match); the V1Writes CHAR(5) case only checks plan shape, which is fine if left as-is.

GitHub will not accept REQUEST_CHANGES on one's own PR, so this is posted as a comment review.

[srielau]

Addressed the review in 6dd42b7: scoped the Hive client-side prune to predicates that mention CHAR/VARCHAR partition keys, tightened the Hive test, dropped the SPARK-58801 parquet catalog/inference block, and renamed the new tests to SPARK-59001.

[srielau]

Re-review of 6dd42b7.

Previous comments are addressed in code:

• Hive client-side prune is now useClientSidePrune = fastFallback || referencesCharVarcharPartitionKey, keyed off predicates that mention a CHAR/VARCHAR partition attribute. Unrelated empty-filter / MetaException fallbacks still honor the conf.
• Hive test is SPARK-59001, asserts METRIC_PARTITIONS_FETCHED == 1 with checkToRDD = false, covers VARCHAR, and checks p = 'a' (no match) vs the padded CHAR literal.
• The SPARK-58801 parquet catalog/inference block is gone. New tests are named SPARK-59001.

The first commit headline is still SPARK-58794; that is fine if dev/merge_spark_pr.py uses the PR title.

Please update the PR description Hive bullet: it still says prune runs when the partition schema contains CHAR/VARCHAR. That is no longer what the code does.

No further code comments from me.

[cloud-fan]

0 blocking, 1 non-blocking, 0 nits.
The V1 write and V2 text changes align with existing string-family behavior, and residual Hive filtering keeps query results correct. One non-blocking pruning gap remains for mixed predicates.

Correctness (1)

• Non-blocking: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala:429: Route mixed predicates that mention a CHAR/VARCHAR partition key through client-side pruning even when convertFilters retains another conjunct. -- see inline

Verification

Statically traced both V1 write callers through Empty2Null, compared V2 text validation with the V1 text implementation, and followed Hive partition lookup through residual filtering. No tests were run as part of this review.

[cloud-fan]

1 addressed, 0 remaining, 1 new to this AI review. (0 newly introduced, 1 late catch, 0 previously raised, 0 unattributed findings.)
0 blocking, 1 non-blocking, 0 nits.
The implementation is sound, with one non-blocking gap in V2 text regression coverage.

Correctness (1)

• Non-blocking: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:2179: Exercise the text test with V2 enabled, because the default V1 routing cannot regress the TextTable.supportsDataType change. -- see inline

Verification

Statically traced text source selection through the default V1 source list, compared V1 and V2 text schema validation, and reviewed the V1 write and Hive pruning changes. No tests were run as part of this review.

[cloud-fan]

Merge Summary:

• merged into master f22e4b4
• merged into branch-4.x 9957fbe

Posted by merge_spark_pr.py