[SPARK-59273][SQL][FOLLOWUP] Restore skipping non-string columns in DataFrameNaFunctions.fillValue - #58646
Closed
pan3793 wants to merge 1 commit into
Closed
[SPARK-59273][SQL][FOLLOWUP] Restore skipping non-string columns in DataFrameNaFunctions.fillValue#58646pan3793 wants to merge 1 commit into
pan3793 wants to merge 1 commit into
Conversation
…ataFrameNaFunctions.fillValue SPARK-59273 widened the fill-value type match in `fillValue` to the whole string family, but the new pattern is no longer exhaustive: `na.fill` with a string value now throws `IllegalArgumentException: StringType is not matched at fillValue` when the schema contains any non-string column. Match the string family while skipping non-string columns, mirroring the NumericType case. Existing tests caught the regression (DataFrameNaFunctionsSuite, SparkConnectProtoSuite, PySpark DataFrameStatTests.test_fillna and its Connect parity test) and pass again. Assisted-by: DeepSeek V4 Flash
|
Thanks for the fix, came to the same conclusion and was planning to fix the issue. |
zhengruifeng
approved these changes
Sep 9, 2026
zhengruifeng
pushed a commit
that referenced
this pull request
Sep 9, 2026
…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>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
SPARK-59273 (#58541) widened the fill-value type match in
DataFrameNaFunctions.fillValuefrom the exactStringTypeto the whole string family: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 _anddf.na.fill(<string>)fails withIllegalArgumentException: StringType is not matched at fillValue. This patch keeps the string-family match but restores the skip for non-string columns:which mirrors the
NumericTypecase above it.Why are the changes needed?
na.fillwith 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:and reddened master CI in
DataFrameNaFunctionsSuite,SparkConnectProtoSuite, and the PySparkDataFrameStatTests.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.fillwith 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 columnsSparkConnectProtoSuite:SPARK-41128: Test fill naDataFrameStatTests.test_fillna,DataFrameStatParityTests.test_fillnaRan locally:
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