uros-b commented on code in PR #59575:
URL: https://github.com/apache/spark/pull/59575#discussion_r4313238227
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionHelper.scala:
##########
@@ -204,6 +205,40 @@ abstract class TypeCoercionHelper {
.getOrElse(implicitCast(e, StringType).getOrElse(e))
}
+ /**
+ * Whether an implicit cast of `from` to `to` narrows a nanosecond-precision
timestamp, also
+ * inside an array, map or struct: a lower-precision timestamp drops
sub-microsecond digits and
+ * TIME drops the date part.
+ */
+ protected def isNanosNarrowing(from: DataType, to: DataType): Boolean =
+ existsNanosPair(from, to) { (nanos, t) =>
+ t.isInstanceOf[TimeType] ||
TimestampFamily.fractionalPrecision(nanos).exists(fp =>
+ TimestampFamily.fractionalPrecision(t).exists(_ < fp))
+ }
+
+ /**
+ * Picks the implicit cast target for a [[TypeCollection]] from the castable
member types, in
+ * member order. A nanosecond-precision timestamp, also nested, skips DATE,
which drops the time
+ * of day.
+ */
+ protected def pickTypeCollectionCast(
Review Comment:
The same DATE-fallback fix exists twice, with two different mechanisms.
#59578 rejects nanos in ApproximatePercentile / HistogramNumeric in two
steps: inputTypes returns the child's own nanos type, and a new
checkInputDataTypes branch rejects it. This PR produces the identical
UNEXPECTED_INPUT_TYPE generically, by skipping DATE in the TypeCollection
fallback.
If both land, one becomes dead weight.
- With this PR in, #59578's overrides never change the outcome.
- With #59578 in, this PR's ruleTest(new ApproximatePercentile(nanos, ...))
passes through the override instead of exercising pickTypeCollectionCast.
- Either way, the golden files carry the same percentile_approx /
approx_percentile / histogram_numeric cases twice.
- The generic rule also covers the internal ApproxCountDistinctForIntervals,
where TIMESTAMP precedes DATE: before this PR, non-ANSI picked TIMESTAMP and
ANSI picked DATE. #59578 doesn't touch it.
Ask: choose one mechanism, say so in both PR bodies, and drop the duplicate
golden cases from the other PR.
##########
docs/sql-ref-datatypes.md:
##########
@@ -56,7 +56,7 @@ Spark SQL and DataFrames support the following data types:
hour, minute, and second. All operations are performed without taking any
time zone into account.
- Note: TIMESTAMP in Spark is a user-specified alias associated with one
of the TIMESTAMP_LTZ and TIMESTAMP_NTZ variations. Users can set the default
timestamp type as `TIMESTAMP_LTZ`(default value) or `TIMESTAMP_NTZ` via the
configuration `spark.sql.timestampType`.
- `TimestampNTZNanosType(precision)` / `TimestampLTZNanosType(precision)`:
Preview nanosecond-capable variants of `TIMESTAMP_NTZ` and `TIMESTAMP_LTZ` with
fractional seconds precision `precision` in `[7, 9]`. Unparameterized
`TIMESTAMP`, `TIMESTAMP_NTZ`, and `TIMESTAMP_LTZ` remain microsecond types. In
schema-driven Dataset/DataFrame conversion, Spark maps `TimestampNTZNanosType`
to `java.time.LocalDateTime` and `TimestampLTZNanosType` to
`java.time.Instant`; values with more sub-micro digits than declared by
`precision` are floor-truncated to that precision. Enable the preview feature
with `SET spark.sql.timestampNanosTypes.enabled=true;` before using these types
in schemas or SQL.
- - Note: Nanosecond precision is only available through the schema-driven
APIs above. Typed encoders derived from
`java.time.Instant`/`java.time.LocalDateTime` (for example `ds.as[T]` for a
case class and `spark.createDataFrame(Seq(...))` without an explicit schema)
bind these types to the microsecond `TIMESTAMP_LTZ`/`TIMESTAMP_NTZ` types, and
typed Scala/Java UDFs such as `udf((i: java.time.Instant) => ...)` coerce their
inputs to microsecond precision at the UDF boundary. Kryo encoders bypass the
SQL type system entirely and likewise do not preserve nanoseconds. To retain
nanoseconds, use a schema that declares
`TimestampLTZNanosType`/`TimestampNTZNanosType` together with the schema-driven
`Row`/`Encoders.row(schema)` path.
+ - Note: Nanosecond precision is only available through the schema-driven
APIs above. Typed encoders derived from
`java.time.Instant`/`java.time.LocalDateTime` (for example `ds.as[T]` for a
case class and `spark.createDataFrame(Seq(...))` without an explicit schema)
bind these types to the microsecond `TIMESTAMP_LTZ`/`TIMESTAMP_NTZ` types, and
typed Scala/Java UDFs such as `udf((i: java.time.Instant) => ...)` take
microsecond inputs, so a nanosecond argument needs an explicit `CAST` to the
microsecond type. Kryo encoders bypass the SQL type system entirely and
likewise do not preserve nanoseconds. To retain nanoseconds, use a schema that
declares `TimestampLTZNanosType`/`TimestampNTZNanosType` together with the
schema-driven `Row`/`Encoders.row(schema)` path.
Review Comment:
The nanos type note still says typed UDFs coerce nanos inputs to
microseconds. After this PR, they reject them in both modes.
The note (SPARK-57808) says typed Scala/Java UDFs such as udf((i:
java.time.Instant) => ...) "coerce their inputs to microsecond precision at the
UDF boundary".
The PR body says a non-ANSI Scala UDF typed as a microsecond timestamp "now
rejects a nanos argument, as ANSI already did". After this PR the note is wrong
in both modes; for ANSI it is already wrong on master. Change it to say that an
explicit CAST is required.
No test covers this user-facing change, and the error class probably differs
from the built-in case (conf med):
- ScalaUDF has no checkInputDataTypes. With no cast inserted, the failure
should come from encoder binding: ResolveEncodersInUDF resolves UpCast(nanos,
TIMESTAMP) and fails canUpCast.
- That is an up-cast error, not the UNEXPECTED_INPUT_TYPE shown for
built-ins. Procedures do report UNEXPECTED_INPUT_TYPE, via Call.checkArgTypes.
Add a typed-UDF test and a procedure test that pin these errors.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AnsiTypeCoercion.scala:
##########
@@ -204,9 +204,12 @@ object AnsiTypeCoercion extends TypeCoercionBase {
// When the target type is `TypeCollection`, there is another branch to
find the
// "closet convertible data type" below.
+ // Unlike store assignment, a nanos timestamp, also nested, is never
implicitly narrowed to
+ // TIME.
Review Comment:
The ANSI TIME block only applies at the top level, so nested TIME targets
still narrow nanos in ANSI mode while non-ANSI rejects them.
isNanosNarrowing only matches a bare nanos from. Take a concrete nested
target such as ArrayType(TimeType(6)):
- The ANSI arm calls Cast.canANSIStoreAssign(ArrayType(nanos),
ArrayType(TIME(6))), which recurses down to (_: DatetimeType, _: DatetimeType)
=> true (Cast.scala:514).
- isNanosNarrowing(ArrayType(..), ArrayType(..)) is false, so the cast is
inserted. Map values and struct fields behave the same way.
This is reachable through concrete nested parameter types:
- a typed Scala UDF taking Seq[java.time.LocalTime] (udfInputToCastType
yields ArrayType(TimeType(6), true));
- a procedure or DSv2 function parameter of type ARRAY<TIME>.
In ANSI mode, such an argument silently loses both the date and the
sub-microsecond digits. Non-ANSI rejects the same input, because its array/map
arms recurse into the guarded scalar arm. So the PR's goal that both modes
agree fails for nested inputs.
Nested precision narrowing is already blocked in ANSI, because
canANSIStoreAssign recurses into the SPARK-57303 check. Only TIME leaks.
Two possible fixes:
- Make isNanosNarrowing recurse through Array/Map/Struct (conf med that this
is sufficient). In ANSI this only adds the nested TIME case.
- Or move the TIME rule into canANSIStoreAssign, which already recurses.
This would also change INSERT into TIME columns, so it is a decision for the
nanos owners.
Either way, add ArrayType(nanos) -> ArrayType(TimestampType) /
ArrayType(TimeType()) cases to the cast matrix for both modes.
--
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]
