sunchao commented on code in PR #56334:
URL: https://github.com/apache/spark/pull/56334#discussion_r3531469848
##########
sql/api/src/main/scala/org/apache/spark/sql/util/ArrowUtils.scala:
##########
@@ -38,6 +38,50 @@ private[sql] object ArrowUtils {
// todo: support more types.
+ /**
+ * Check if a Spark DataType is supported by Arrow. This recursively checks
complex types
+ * (Array, Struct, Map).
+ *
+ * Note: This checks compatibility with toArrowField(), not toArrowType().
Types like
+ * GeometryType, GeographyType, and VariantType are not supported by
toArrowType() (which only
+ * handles primitive Arrow types), but ARE supported by toArrowField() which
converts them to
+ * Arrow Struct representations with metadata. Since Arrow cache uses
toArrowField() via
+ * toArrowSchema() to create the schema, these types are supported.
+ */
+ def isSupportedByArrow(dt: DataType): Boolean = {
+ dt match {
+ // Primitive types
+ case BooleanType | ByteType | ShortType | IntegerType | LongType |
FloatType | DoubleType |
+ _: StringType | BinaryType | NullType =>
+ true
+
+ // Decimal
+ case _: DecimalType => true
+
+ // Temporal types
+ case DateType | TimestampType | TimestampNTZType | _: TimeType => true
+
+ // Interval types
+ case _: YearMonthIntervalType | _: DayTimeIntervalType |
CalendarIntervalType => true
Review Comment:
[P2] Please narrow the `CalendarInterval` overflow translation to the actual
interval conversion. The new schema-wide catch wraps the entire batch loop
whenever any interval field exists, and `SparkArithmeticException` extends
`ArithmeticException`. I reproduced this with one partition containing two rows
under ANSI mode: `range(0, 2, 1, 1).selectExpr("1 / (1 - id) AS bad",
"make_interval() AS i").cache().count()`. Without the harmless interval column,
the second row fails as `DIVIDE_BY_ZERO`; with it, the same failure is caught
inside the batch loop and rethrown as `DATETIME_OVERFLOW` claiming a
CalendarInterval overflow. Adding an unrelated interval column must not change
the error condition or SQLSTATE. Please catch the raw `Math.multiplyExact`
failure at `IntervalMonthDayNanoWriter`, or preserve structured/non-interval
arithmetic exceptions unchanged.
##########
sql/api/src/main/scala/org/apache/spark/sql/util/ArrowUtils.scala:
##########
@@ -38,6 +38,50 @@ private[sql] object ArrowUtils {
// todo: support more types.
+ /**
+ * Check if a Spark DataType is supported by Arrow. This recursively checks
complex types
+ * (Array, Struct, Map).
+ *
+ * Note: This checks compatibility with toArrowField(), not toArrowType().
Types like
+ * GeometryType, GeographyType, and VariantType are not supported by
toArrowType() (which only
+ * handles primitive Arrow types), but ARE supported by toArrowField() which
converts them to
+ * Arrow Struct representations with metadata. Since Arrow cache uses
toArrowField() via
+ * toArrowSchema() to create the schema, these types are supported.
+ */
+ def isSupportedByArrow(dt: DataType): Boolean = {
+ dt match {
+ // Primitive types
+ case BooleanType | ByteType | ShortType | IntegerType | LongType |
FloatType | DoubleType |
+ _: StringType | BinaryType | NullType =>
+ true
+
+ // Decimal
+ case _: DecimalType => true
+
+ // Temporal types
+ case DateType | TimestampType | TimestampNTZType | _: TimeType => true
Review Comment:
[P2] The follow-up support is still not parity with the default cache for
valid nanos-timestamp values outside Arrow's signed-INT64 epoch-nanos window.
Spark defines both `TimestampNTZNanosType` and `TimestampLTZNanosType` over
years 0001-9999, and the default cache stores the `(epochMicros,
nanosWithinMicro)` pair losslessly. This serializer instead routes non-Arrow
input through `ArrowWriter`, which packs the value into one `Long`. I
reproduced this with `LocalDateTime.of(9999, 12, 31, 23, 59, 59, 999999999)`:
the default serializer materializes the row, while the Arrow serializer fails
with `DATETIME_OVERFLOW` because only roughly 1677-2262 is representable. There
is no fallback after selecting this serializer, and the guide omits this
restriction while claiming coverage of every type supported by the default
cache. Please either use a lossless cache representation or document the
reduced domain beside `CalendarIntervalType` and narrow the parity claim.
--
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]
