dongjoon-hyun opened a new pull request, #60037:
URL: https://github.com/apache/spark/pull/60037

   ### What changes were proposed in this pull request?
   
   This PR aims to fix a correctness issue of file source partition values. 
`PartitioningUtils.castPartValueToDesiredType` now parses a `TINYINT` or 
`SMALLINT` partition value with `java.lang.Byte.parseByte` and 
`java.lang.Short.parseShort` instead of narrowing the `Int` that 
`Integer.parseInt` returns.
   
   ```scala
   -    case ByteType => Integer.parseInt(value).toByte
   -    case ShortType => Integer.parseInt(value).toShort
   +    case ByteType => JByte.parseByte(value)
   +    case ShortType => JShort.parseShort(value)
   ```
   
   Both methods accept the same strings as `Integer.parseInt` and throw 
`NumberFormatException` for a value out of the range of the type, as 
`Integer.parseInt` does for `INT`. The two callers already handle it.
   - `parsePartitions` fails with `INVALID_PARTITION_VALUE`, or returns `NULL` 
when `spark.sql.sources.validatePartitionColumns` is `false`.
   - `removeLeadingZerosFromNumberTypePartition`, which normalizes a partition 
value for its path on write, throws the exception, or keeps the value as it is 
when `spark.sql.sources.validatePartitionColumns` is `false`.
   
   This is related to #60003 (SPARK-60797), which fixed the `DECIMAL` case of 
the same function.
   
   ### Why are the changes needed?
   
   This is a correctness issue. When a file source reads a partition directory 
with a user-specified `TINYINT` or `SMALLINT` type, it returns a wrong value 
without any error if the value is out of the range of the type, regardless of 
the ANSI mode. A value out of the range of `INT` already fails.
   
   ```scala
   spark.range(1).selectExpr("id AS v").write.parquet(s"$base/t1/b=300")
   spark.range(1).selectExpr("id AS v").write.parquet(s"$base/t2/s=70000")
   spark.range(1).selectExpr("id AS v").write.parquet(s"$base/t3/i=99999999999")
   
   // It should fail with INVALID_PARTITION_VALUE, but it returns [44].
   spark.read.schema("v LONG, b 
TINYINT").parquet(s"$base/t1").select("b").collect()
   
   // It should fail with INVALID_PARTITION_VALUE, but it returns [4464].
   spark.read.schema("v LONG, s 
SMALLINT").parquet(s"$base/t2").select("s").collect()
   
   // An out-of-range INT value already fails.
   spark.read.schema("v LONG, i INT").parquet(s"$base/t3").select("i").collect()
   // [INVALID_PARTITION_VALUE] Failed to cast value '99999999999' to data type 
"INT" for partition column `i`. ...
   ```
   
   `castPartValueToDesiredType` parses the value as an `Int` and narrows it 
with `toByte` or `toShort`, which keeps only the low-order bits. This affects 
queries as follows.
   - Partition pruning compares the wrapped value, so `WHERE b = 44` keeps the 
directory `b=300`.
   - A catalog table that does not track its partitions 
(`spark.sql.hive.manageFilesourcePartitions=false`) reads the directories in 
the same way and returns `44`.
   - On write, the path of a static partition is computed from the wrapped 
value. With `spark.sql.legacy.skipTypeValidationOnAlterPartition=true` and 
`spark.sql.storeAssignmentPolicy=LEGACY`, `INSERT INTO t PARTITION (b=300)` of 
no rows adds the partition `b=44` to the catalog and creates its directory.
   
   Reading partitions tracked in the catalog is not affected, because 
`CatalogTablePartition.toRow` uses `Cast`.
   
   This issue has existed since SPARK-40212 in 
[v3.4.0](https://github.com/apache/spark/releases/tag/v3.4.0) (2023-04-07), 
which added `toByte` and `toShort` to fix a `ClassCastException` on every 
`TINYINT` and `SMALLINT` partition value. I reproduced the wrong values with 
[v4.0.4](https://github.com/apache/spark/releases/tag/v4.0.4) (2026-07-12), 
[v4.1.3](https://github.com/apache/spark/releases/tag/v4.1.3) (2026-07-11), 
[v4.2.0](https://github.com/apache/spark/releases/tag/v4.2.0) (2026-07-11) and 
[v4.3.0](https://github.com/apache/spark/releases/tag/v4.3.0) (2026-10-27).
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. These are the results of reading each directory with the user-specified 
type. The results before this PR are the same in both ANSI modes.
   
   | Directory | Type | Before | After |
   |---|---|---|---|
   | `b=300` | `TINYINT` | `44` | `INVALID_PARTITION_VALUE` |
   | `b=-129` | `TINYINT` | `127` | `INVALID_PARTITION_VALUE` |
   | `s=70000` | `SMALLINT` | `4464` | `INVALID_PARTITION_VALUE` |
   | `b=127`, `b=007`, `b=+5` | `TINYINT` | `127`, `7`, `5` | `127`, `7`, `5` |
   
   A value out of range now fails regardless of `spark.sql.ansi.enabled`, and 
it becomes `NULL` when `spark.sql.sources.validatePartitionColumns` is `false`. 
This is how an `INT` value already behaves, and the SQL migration guide has 
stated it since Spark 3.0: "An exception is thrown if the validation fails".
   
   On write, an out-of-range value now throws `NumberFormatException` when 
Spark computes the path of the partition, or it is kept as it is when 
`spark.sql.sources.validatePartitionColumns` is `false`, as an out-of-range 
`INT` value is. So the empty insert above now fails, or adds the partition 
`b=300`. This also changes a table whose catalog tracks such a partition, e.g. 
one that `MSCK REPAIR TABLE` added from the directory `b=300`. An insert that 
does not exclude that partition with a static partition value, e.g. `INSERT 
INTO t SELECT 1, 1`, succeeded before and now fails with 
`NumberFormatException` unless `spark.sql.sources.validatePartitionColumns` is 
`false`. Reading that partition does not change: it fails with 
`CAST_INVALID_INPUT` in ANSI mode and returns `NULL` otherwise.
   
   The SQL migration guide describes this change.
   
   ### How was this patch tested?
   
   Pass the CIs with the newly added test cases. Both of them fail without the 
fix.
   - `FileIndexSuite` reads `a=300` as `TINYINT` and `a=70000` as `SMALLINT`, 
with `spark.sql.sources.validatePartitionColumns` on and off.
   - `PartitioningUtilsSuite` checks 
`removeLeadingZerosFromNumberTypePartition` with the same values.
   
   ```
   $ build/sbt "sql/testOnly *FileIndexSuite *PartitioningUtilsSuite -- -z 
SPARK-60818"
   ```
   
   I also ran a temporary suite, which is not part of this PR. It compares the 
results before and after the fix for in-range and out-of-range values, in both 
ANSI modes, with `spark.sql.sources.validatePartitionColumns` on and off, and 
through both V1 and V2. Only the out-of-range `TINYINT` and `SMALLINT` values 
changed. It also covers catalog partitions added by `MSCK REPAIR TABLE` and 
`ALTER TABLE ADD PARTITION`, and static partition inserts.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Opus 5.5


-- 
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]

Reply via email to