[SPARK-59273][SQL] Complete CHAR/VARCHAR support at core execution boundaries by srielau · Pull Request #58541 · apache/spark

srielau · 2026-09-05T05:05:31Z

What changes were proposed in this pull request?

When spark.sql.charVarchar.standardSemantics.enabled is true, CharType and VarcharType are first-class StringType subtypes. Several execution-boundary matchers still used exact StringType (or an explicit CHAR/VARCHAR reject), so those paths failed or skipped constrained string columns.

This patch treats CHAR/VARCHAR as the string family at:

JDBC getters/setters and JDBC array element conversion
JDBC schema inference (CHAR/VARCHAR keep first-class types when standard semantics is on, even if charVarcharAsString is also set)
File partition value decoding
RowToColumnConverter
DataFrame.na.fill for string replacement values
ANALYZE TABLE ... FOR COLUMNS (string-family stats)

Read-side CHAR padding and VARCHAR overflow still come from existing CAST / ApplyCharTypePadding paths rather than being reimplemented in each converter.

Why are the changes needed?

With first-class CHAR/VARCHAR, JDBC scans/writes, file-only partition discovery, columnar conversion, na.fill("..."), and column stats currently throw or silently ignore those columns. That blocks enabling standard semantics.

JIRA: https://issues.apache.org/jira/browse/SPARK-59273 (subtask of SPARK-58794)

Does this PR introduce any user-facing change?

Yes, when spark.sql.charVarchar.standardSemantics.enabled is true (still default false):

JDBC read/write of CHAR/VARCHAR (including arrays) no longer fails with an unsupported JDBC type.
File partition columns declared as CHAR/VARCHAR can be decoded; CHAR is padded on scan and oversize VARCHAR fails with EXCEED_LIMIT_LENGTH.
Columnar row-to-column conversion accepts CHAR/VARCHAR.
df.na.fill("x") fills null CHAR/VARCHAR columns (CHAR values are padded by CAST).
ANALYZE TABLE ... FOR COLUMNS collects string-family stats on CHAR/VARCHAR instead of rejecting them.

How was this patch tested?

Added/extended unit tests:

JDBCSuite: read CHAR/VARCHAR and arrays; write CHAR/VARCHAR; standard semantics wins over charVarcharAsString in schema inference
ParquetV1PartitionDiscoverySuite / ParquetV2PartitionDiscoverySuite: CHAR/VARCHAR partition values and oversize VARCHAR
RowToColumnConverterSuite: CHAR/VARCHAR and nested CHAR arrays
DataFrameNaFunctionsSuite: na.fill on CHAR/VARCHAR
StatisticsCollectionSuite: ANALYZE TABLE ... FOR COLUMNS on CHAR/VARCHAR

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

Generated-by: Cursor Grok 4.6

…undaries Treat first-class CharType/VarcharType as the string family in JDBC getters/setters, partition decoding, row-to-column conversion, na.fill, and ANALYZE COLUMN stats so standardSemantics no longer fails at these boundaries.

cloud-fan

Review summary

The execution-boundary changes and their direct tests are otherwise consistent, including the corrected JDBC configuration precedence. Before this is ready, the CBO consumers of column statistics need to handle CHAR/VARCHAR as members of the string family so enabling the new ANALYZE path cannot turn valid query planning into a MatchError.

Findings

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

Blocking (P1)

Handle constrained strings in CBO after collecting their statistics — sql/core/src/main/scala/org/apache/spark/sql/execution/command/AnalyzeColumnCommand.scala:145 — see inline.

…er estimation

cloud-fan

Review summary

The previously reported CBO MatchError after collecting CHAR/VARCHAR statistics is fixed, and the execution-boundary changes are consistent with the shared first-class string-family contract. Two non-blocking follow-ups remain: add CHAR/VARCHAR to the owning join-estimation type matrix so the widened interval and size paths cannot regress untested, and correct one newly added comment that uses MatchError as a verb.

Findings

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

Non-blocking (P2)

Exercise CHAR/VARCHAR join estimation — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/ValueInterval.scala:56 — see inline.

Nit (P3)

Use MatchError as an exception, not a verb — sql/core/src/test/scala/org/apache/spark/sql/StatisticsCollectionSuite.scala:189 — see inline.

Re-review status

Prior AI findings: 1 addressed, 0 still present; additional unresolved findings in this review: 2.

New attribution: 2 newly introduced, 0 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

dtenedor

LGTM, merging to master + 4.x

…undaries ### What changes were proposed in this pull request? When `spark.sql.charVarchar.standardSemantics.enabled` is true, `CharType` and `VarcharType` are first-class `StringType` subtypes. Several execution-boundary matchers still used exact `StringType` (or an explicit CHAR/VARCHAR reject), so those paths failed or skipped constrained string columns. This patch treats CHAR/VARCHAR as the string family at: - JDBC getters/setters and JDBC array element conversion - JDBC schema inference (`CHAR`/`VARCHAR` keep first-class types when standard semantics is on, even if `charVarcharAsString` is also set) - File partition value decoding - `RowToColumnConverter` - `DataFrame.na.fill` for string replacement values - `ANALYZE TABLE ... FOR COLUMNS` (string-family stats) Read-side CHAR padding and VARCHAR overflow still come from existing CAST / `ApplyCharTypePadding` paths rather than being reimplemented in each converter. ### Why are the changes needed? With first-class CHAR/VARCHAR, JDBC scans/writes, file-only partition discovery, columnar conversion, `na.fill("...")`, and column stats currently throw or silently ignore those columns. That blocks enabling standard semantics. JIRA: https://issues.apache.org/jira/browse/SPARK-59273 (subtask of SPARK-58794) ### Does this PR introduce _any_ user-facing change? Yes, when `spark.sql.charVarchar.standardSemantics.enabled` is true (still default false): - JDBC read/write of CHAR/VARCHAR (including arrays) no longer fails with an unsupported JDBC type. - File partition columns declared as CHAR/VARCHAR can be decoded; CHAR is padded on scan and oversize VARCHAR fails with `EXCEED_LIMIT_LENGTH`. - Columnar row-to-column conversion accepts CHAR/VARCHAR. - `df.na.fill("x")` fills null CHAR/VARCHAR columns (CHAR values are padded by CAST). - `ANALYZE TABLE ... FOR COLUMNS` collects string-family stats on CHAR/VARCHAR instead of rejecting them. ### How was this patch tested? Added/extended unit tests: - `JDBCSuite`: read CHAR/VARCHAR and arrays; write CHAR/VARCHAR; standard semantics wins over `charVarcharAsString` in schema inference - `ParquetV1PartitionDiscoverySuite` / `ParquetV2PartitionDiscoverySuite`: CHAR/VARCHAR partition values and oversize VARCHAR - `RowToColumnConverterSuite`: CHAR/VARCHAR and nested CHAR arrays - `DataFrameNaFunctionsSuite`: `na.fill` on CHAR/VARCHAR - `StatisticsCollectionSuite`: `ANALYZE TABLE ... FOR COLUMNS` on CHAR/VARCHAR ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Cursor Grok 4.6 Closes #58541 from srielau/serge-rielau_data/SPARK-59273. Authored-by: Serge Rielau <serge@rielau.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com> (cherry picked from commit 51f54b0) Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

dtenedor · 2026-09-08T19:12:42Z

Merge Summary:

merged into master 51f54b0
merged into branch-4.x a9fd0a1

Posted by merge_spark_pr.py

ulysses-you · 2026-09-09T03:05:38Z

Seems CI did not pass and related to this change:

  [info] - SPARK-41128: Test fill na *** FAILED *** (45 milliseconds)
  [info]   java.lang.IllegalArgumentException: StringType is not matched at fillValue
  [info]   at org.apache.spark.sql.classic.DataFrameNaFunctions.$anonfun$fillValue$3(DataFrameNaFunctions.scala:255)
  [info]   at scala.collection.immutable.List.map(List.scala:236)
  [info]   at scala.collection.immutable.List.map(List.scala:79)
  [info]   at org.apache.spark.sql.classic.DataFrameNaFunctions.fillValue(DataFrameNaFunctions.scala:249)
  [info]   at org.apache.spark.sql.classic.DataFrameNaFunctions.fill(DataFrameNaFunctions.scala:56)
  [info]   at org.apache.spark.sql.connect.planner.SparkConnectProtoSuite.$anonfun$new$34(SparkConnectProtoSuite.scala:478)
  [info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
  [info]   at org.scalatest.enablers.Timed$$anon$1.timeoutAfter(Timed.scala:127)
  [info]   at org.scalatest.concurrent.TimeLimits$.failAfterImpl(TimeLimits.scala:282)
  [info]   at org.scalatest.concurrent.TimeLimits.failAfter(TimeLimits.scala:231)
  [info]   at org.scalatest.concurrent.TimeLimits.failAfter$(TimeLimits.scala:230)
  [info]   at org.apache.spark.SparkFunSuite.failAfter(SparkFunSuite.scala:33)

pan3793 · 2026-09-09T03:07:24Z

opened a fix #58646 @dtenedor @srielau @ulysses-you

…ataFrameNaFunctions.fillValue ### What changes were proposed in this pull request? SPARK-59273 (#58541) widened the fill-value type match in `DataFrameNaFunctions.fillValue` from the exact `StringType` to the whole string family: ```scala - case (StringType, dt) => dt == StringType + case (StringType, _: StringType) => true ``` That pattern is no longer exhaustive for a string fill value: when the schema contains any non-string column, that column falls through to the throwing `case _` and `df.na.fill(<string>)` fails with `IllegalArgumentException: StringType is not matched at fillValue`. This patch keeps the string-family match but restores the skip for non-string columns: ```scala case (StringType, dt) => dt.isInstanceOf[StringType] ``` which mirrors the `NumericType` case above it. ### Why are the changes needed? `na.fill` with a string value must fill only string-family columns and leave other columns untouched; a schema with any non-string column must not throw. The regression breaks basic usage such as: ```scala Seq[(String, Integer)]((null, null)).toDF("name", "age").na.fill("unknown") ``` and reddened master CI in `DataFrameNaFunctionsSuite`, `SparkConnectProtoSuite`, and the PySpark `DataFrameStatTests.test_fillna` / `DataFrameStatParityTests.test_fillna`. SPARK-59273 has not shipped in a release. ### Does this PR introduce _any_ user-facing change? No. It restores on master the pre-SPARK-59273 behavior of `DataFrameNaFunctions.fill` with a string value. ### How was this patch tested? Existing tests already exercise mixed-type schemas and both behaviors, and failed on master after SPARK-59273: - `DataFrameNaFunctionsSuite`: `fill`, `fill with col(*)`, `fill with nested columns` - `SparkConnectProtoSuite`: `SPARK-41128: Test fill na` - PySpark `DataFrameStatTests.test_fillna`, `DataFrameStatParityTests.test_fillna` Ran locally: ``` build/sbt 'sql/testOnly *DataFrameNaFunctionsSuite' build/sbt 'connect/testOnly *SparkConnectProtoSuite' ``` Both suites pass; no new tests added because the failing cases above cover the regression. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: DeepSeek V4 Flash Closes #58646 from pan3793/spark-59273-nafill-followup. Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>

…ataFrameNaFunctions.fillValue ### What changes were proposed in this pull request? SPARK-59273 (#58541) widened the fill-value type match in `DataFrameNaFunctions.fillValue` from the exact `StringType` to the whole string family: ```scala - case (StringType, dt) => dt == StringType + case (StringType, _: StringType) => true ``` That pattern is no longer exhaustive for a string fill value: when the schema contains any non-string column, that column falls through to the throwing `case _` and `df.na.fill(<string>)` fails with `IllegalArgumentException: StringType is not matched at fillValue`. This patch keeps the string-family match but restores the skip for non-string columns: ```scala case (StringType, dt) => dt.isInstanceOf[StringType] ``` which mirrors the `NumericType` case above it. ### Why are the changes needed? `na.fill` with a string value must fill only string-family columns and leave other columns untouched; a schema with any non-string column must not throw. The regression breaks basic usage such as: ```scala Seq[(String, Integer)]((null, null)).toDF("name", "age").na.fill("unknown") ``` and reddened master CI in `DataFrameNaFunctionsSuite`, `SparkConnectProtoSuite`, and the PySpark `DataFrameStatTests.test_fillna` / `DataFrameStatParityTests.test_fillna`. SPARK-59273 has not shipped in a release. ### Does this PR introduce _any_ user-facing change? No. It restores on master the pre-SPARK-59273 behavior of `DataFrameNaFunctions.fill` with a string value. ### How was this patch tested? Existing tests already exercise mixed-type schemas and both behaviors, and failed on master after SPARK-59273: - `DataFrameNaFunctionsSuite`: `fill`, `fill with col(*)`, `fill with nested columns` - `SparkConnectProtoSuite`: `SPARK-41128: Test fill na` - PySpark `DataFrameStatTests.test_fillna`, `DataFrameStatParityTests.test_fillna` Ran locally: ``` build/sbt 'sql/testOnly *DataFrameNaFunctionsSuite' build/sbt 'connect/testOnly *SparkConnectProtoSuite' ``` Both suites pass; no new tests added because the failing cases above cover the regression. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: DeepSeek V4 Flash Closes #58646 from pan3793/spark-59273-nafill-followup. Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com> (cherry picked from commit 3d25204) Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>

srielau commented Sep 5, 2026

View reviewed changes

Comment thread sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala Outdated

[SPARK-59273][SQL][FOLLOWUP] Honor all first-class CHAR/VARCHAR modes

d3a9c7f

cloud-fan reviewed Sep 5, 2026

View reviewed changes

Comment thread sql/core/src/main/scala/org/apache/spark/sql/execution/command/AnalyzeColumnCommand.scala

[SPARK-59273][SQL][FOLLOWUP] Treat CHAR/VARCHAR as STRING in CBO filt…

42e1b25

…er estimation

srielau requested a review from cloud-fan September 7, 2026 16:52

cloud-fan approved these changes Sep 8, 2026

View reviewed changes

Comment thread ...c/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/ValueInterval.scala

Comment thread sql/core/src/test/scala/org/apache/spark/sql/StatisticsCollectionSuite.scala Outdated

[SPARK-59273][SQL] Add CHAR/VARCHAR join estimation coverage

91e5bc8

dtenedor approved these changes Sep 8, 2026

View reviewed changes

dtenedor closed this in 51f54b0 Sep 8, 2026

pan3793 mentioned this pull request Sep 9, 2026

[SPARK-59273][SQL][FOLLOWUP] Restore skipping non-string columns in DataFrameNaFunctions.fillValue #58646

Closed

Navigation Menu

Uh oh!

[SPARK-59273][SQL] Complete CHAR/VARCHAR support at core execution boundaries - #58541

[SPARK-59273][SQL] Complete CHAR/VARCHAR support at core execution boundaries#58541
srielau wants to merge 4 commits into
apache:masterfrom
srielau:serge-rielau_data/SPARK-59273

srielau commented Sep 5, 2026

Uh oh!

Uh oh!

cloud-fan left a comment

Uh oh!

Uh oh!

cloud-fan left a comment

Uh oh!

Uh oh!

Uh oh!

dtenedor left a comment

Uh oh!

dtenedor commented Sep 8, 2026

Uh oh!

ulysses-you commented Sep 9, 2026

Uh oh!

pan3793 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

srielau commented Sep 5, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

Uh oh!

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Review summary

Findings

Blocking (P1)

Uh oh!

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Review summary

Findings

Non-blocking (P2)

Nit (P3)

Re-review status

Remaining prior AI findings

Uh oh!

Uh oh!

Uh oh!

dtenedor left a comment

Choose a reason for hiding this comment

Uh oh!

dtenedor commented Sep 8, 2026

Uh oh!

ulysses-you commented Sep 9, 2026

Uh oh!

pan3793 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants