dongjoon-hyun commented on code in PR #60083:
URL: https://github.com/apache/spark/pull/60083#discussion_r4470771232
##########
docs/sql-migration-guide.md:
##########
@@ -31,6 +31,7 @@ license: |
- Since Spark 4.4, when a file source reads a partition column with a
user-specified `DECIMAL` type from directory names, it converts each value to
the precision and scale of that type, rounding half up as `CAST` does.
Previously, the vectorized Parquet and ORC readers applied the declared scale
to the digits in the directory name without an error, for example `d=1.5` read
as `DECIMAL(10,2)` returned `0.15`. Data derived from affected query results
may need to be recomputed. A value that does not fit the declared precision now
fails with `INVALID_PARTITION_VALUE` regardless of `spark.sql.ansi.enabled`.
Previously, those readers returned a wrong value and the other readers returned
`NULL`. To get `NULL` for such a value, set
`spark.sql.sources.validatePartitionColumns` to `false`. Partitions tracked in
the catalog are not affected.
- Since Spark 4.4, when a file source reads a partition column with a
user-specified `TINYINT` or `SMALLINT` type from directory names, a value out
of the range of that type fails with `INVALID_PARTITION_VALUE` regardless of
`spark.sql.ansi.enabled`, as it does for `INT`. Previously, the value wrapped
around without an error, for example `b=300` read as `TINYINT` returned `44`.
Data derived from affected query results may need to be recomputed. To get
`NULL` for such a value, set `spark.sql.sources.validatePartitionColumns` to
`false`. Reading partitions tracked in the catalog is not affected, but writing
to a table whose catalog tracks such a partition, for example one added by
`MSCK REPAIR TABLE`, can now fail with `NumberFormatException` unless
`spark.sql.sources.validatePartitionColumns` is `false`.
- Since Spark 4.4, `size` and `cardinality` of a map built by `map(...)`
return the number of entries of the built map. Previously, the optimizer could
return the number of key-value pairs instead, for example `size(map(1, 'a', 1,
'b'))` returned `2` instead of `1` when `spark.sql.mapKeyDedupPolicy` is
`LAST_WIN`. Data derived from affected query results may need to be recomputed.
Under the default `EXCEPTION` policy, such a query now fails with
`DUPLICATED_MAP_KEY`, and a NULL key fails with `NULL_MAP_KEY`, as building the
map does. This also applies to a map inside `array(...)`, for example
`size(array(map(k, 1)))`.
+- Since Spark 4.4, when `spark.sql.ansi.enabled` is `false`, `size` and
`cardinality` of an array built by `array(...)` evaluate the LEGACY casts in it
that can fail, such as a cast from an interval to `TINYINT`, from a string to
an interval, or to `VARCHAR(n)`. Previously, the optimizer could return the
number of elements without evaluating such a cast. For example, for an
`INTERVAL SECOND` column `i` with the value `INTERVAL '1000' SECOND`,
`size(array(CAST(i AS TINYINT)))` returned `1` instead of failing with
`CAST_OVERFLOW`, and `try_add(DATE'2020-01-01', size(array(CAST(i AS
TINYINT))))` returned `2020-01-02` instead of `NULL`. Data derived from
affected query results may need to be recomputed.
Review Comment:
Thanks. I applied your wording, and the third example in the description now
mentions `spark.sql.charVarchar.standardSemantics.enabled` too.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala:
##########
@@ -594,6 +594,20 @@ object Cast extends QueryErrorsBase {
case _ => false
}
+ /**
+ * Returns true if a LEGACY cast from `from` type to `to` type never fails
at runtime. Among
+ * numeric, boolean and string types, it wraps around on overflow or returns
null for an invalid
+ * input instead. Other LEGACY casts can fail, for example from or to an
interval and to
+ * CHAR/VARCHAR. The patterns matched reflect the current implementation in
the Cast node and
+ * must stay in sync with it.
Review Comment:
Thanks, that is more accurate. I used your wording in the Scaladoc and the
same in the description.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantFoldingSuite.scala:
##########
@@ -342,6 +342,23 @@ class ConstantFoldingSuite extends PlanTest {
}
}
+ test("SPARK-60868: Do not fold the size of CreateArray over a LEGACY cast
that can fail") {
+ val t = LocalRelation($"i".int, $"d".dayTimeInterval())
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") {
+ Seq(
+ // This cannot fail, as in SPARK-37392.
+ Cast($"i", StringType) -> true,
+ // These fail with CAST_OVERFLOW and EXCEED_LIMIT_LENGTH.
+ Cast($"d", ByteType) -> false,
+ Cast($"i", VarcharType(2)) -> false).foreach { case (cast, foldable) =>
Review Comment:
Thanks for the mutation check. I added the TRY row and the nested cast row,
and relaxed the assert. Each of your two mutants now fails the test, and the
test passes with this PR. I did not add the string-to-interval row, because it
takes the same `case _ => false` branch of `Cast.isNoThrowLegacyCast` as
`Cast($"d", ByteType)`.
--
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]
