This is an automated email from the ASF dual-hosted git repository.
MaxGekk pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new a27f11d998d3 [SPARK-57338][SQL] Render external values in Row JSON via
formatExternal
a27f11d998d3 is described below
commit a27f11d998d3b19619e0c95f95b3833b40d6abd7
Author: Maxim Gekk <[email protected]>
AuthorDate: Wed Jun 10 08:37:26 2026 +0200
[SPARK-57338][SQL] Render external values in Row JSON via formatExternal
### What changes were proposed in this pull request?
This PR fixes `Row.json` / `Row.prettyJson` for `TIME` (`TimeType`) columns
by rendering the row's external value through the Types Framework's
`formatExternal` instead of the internal-value `format`.
- `Row.jsonValue`'s `toJson` now renders a framework type's value via
`TypeApiOps(dt).formatExternal(value)`. A framework type either returns a
rendered string or raises its own error; types outside the framework fall back
to the legacy `toJsonDefault`.
- `TimeTypeApiOps.formatExternal` now formats the external
`java.time.LocalTime` directly through the time formatter. It previously
converted to nanos-of-day first; that was an identity round-trip
(`localTimeToNanos` then `nanosToLocalTime`), so the rendered output is
unchanged.
- The nanosecond timestamp ops override the single-arg
`formatExternal(value)` to raise
`UNSUPPORTED_FEATURE.TIMESTAMP_NANOS_TO_STRING` directly (the same error their
`format` raises), so Row JSON keeps reporting that the rendering is unsupported
instead of mis-rendering the external value. With that, `Row.toJson` no longer
needs to fall back to the internal-value `format`.
- The nanosecond timestamp ops also override the two-arg
`formatExternal(value, nested)` to return `None`. That overload is the entry
point used by `HiveResult.toHiveString`, which is zone-aware and renders
nanosecond timestamps through its own default formatter; returning `None` lets
it fall through to that path instead of inheriting the single-arg overload's
unsupported-rendering error. Without this override the Hive/SQL output path
would regress (it previously relied on `formatExte [...]
- `toJsonDefault` now also handles the external `java.time.LocalTime` for a
`TimeType` column, so `Row.json` renders a TIME column even with the Types
Framework off (the production default), where `TypeApiOps` returns `None` and
the value falls into the legacy path. This mirrors
`HiveResult.toHiveStringDefault`'s existing legacy fallback; previously this
path threw `FAILED_ROW_TO_JSON`.
- Updated the `TypeApiOps` trait scaladoc for the two `formatExternal`
overloads to name both consumers (single-arg = Row JSON, two-arg =
`HiveResult`) and define the `Some` / `None` / throw contract.
Root cause: a public `Row` holds *external* values (e.g.
`java.time.LocalTime` for `TimeType`), but `toJson` called the internal-value
`format`, which does `value.asInstanceOf[Long]` and threw `ClassCastException`.
On the framework-off legacy path the external `LocalTime` instead hit the
`toJsonDefault` fallthrough and threw `FAILED_ROW_TO_JSON`.
### Why are the changes needed?
With the Types Framework enabled, serializing a row that has a `TIME`
column to JSON fails:
```scala
import java.time.LocalTime
import org.apache.spark.sql.catalyst.expressions.GenericRowWithSchema
import org.apache.spark.sql.types._
val row = new GenericRowWithSchema(
Array(LocalTime.of(12, 13, 14)),
new StructType().add("a", TimeType()))
row.json
```
throws:
```
java.lang.ClassCastException: class java.time.LocalTime cannot be cast to
class java.lang.Long
```
because the JSON path passed the external `LocalTime` to the internal-value
`format`. The framework already exposes `formatExternal` (the external-value
entry point, as `HiveResult` uses); Row JSON should use it.
The same `Row.json`-on-`TIME` failure also exists with the Types Framework
off (the production default): there `TypeApiOps` returns `None`, the external
`LocalTime` falls into `toJsonDefault`, which only matched the internal `Long`,
and the row failed with `FAILED_ROW_TO_JSON` - even though `HiveResult`
rendered the same value fine via its legacy fallback. This PR fixes both paths.
This also closes the coverage gap noted in #56355: `Row.jsonValue` was not
exercised for framework types.
### Does this PR introduce _any_ user-facing change?
Yes. `Row.json` / `Row.prettyJson` on a `TIME` column previously failed
(with `ClassCastException` under the Types Framework, or `FAILED_ROW_TO_JSON`
with it off); it now renders the value regardless of the flag, e.g.:
```
{"a":"12:13:14"}
```
The nanosecond timestamp types are unaffected: Row JSON keeps raising
`UNSUPPORTED_FEATURE.TIMESTAMP_NANOS_TO_STRING` (via the single-arg
`formatExternal`), and the Hive/SQL output path keeps rendering nanosecond
timestamps as before (via the two-arg `formatExternal` returning `None`).
### How was this patch tested?
Added `RowJsonSuite` tests (tagged `SPARK-57338`):
- `TIME` column rendering: plain, fractional (microsecond resolution,
trailing zeros trimmed), and midnight.
- Rendering is independent of the `TimeType` precision.
- A null `TIME` column renders as JSON `null`.
- `TIME` column rendering on the legacy path with the Types Framework
disabled (plain, fractional, and null).
- A guard that nanosecond timestamp columns still raise
`UNSUPPORTED_FEATURE.TIMESTAMP_NANOS_TO_STRING` in Row JSON.
I also confirmed the new `TIME` test fails on the pre-fix code with the
expected `ClassCastException`, and re-ran the affected suites (`RowJsonSuite`,
`HiveResultSuite`, and `SQLQueryTestSuite` for `timestamp-ltz-nanos.sql` /
`timestamp-ntz-nanos.sql`) to confirm the Hive/SQL nanos rendering path stays
green. `./dev/scalastyle` passes.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor (Claude Opus 4.8)
Closes #56392 from MaxGekk/row-json-time-formatexternal.
Authored-by: Maxim Gekk <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
(cherry picked from commit 844f6f0dbdcd85c804b87cf08585d6f3b626f621)
Signed-off-by: Max Gekk <[email protected]>
---
.../src/main/scala/org/apache/spark/sql/Row.scala | 15 +++-
.../spark/sql/types/ops/TimeTypeApiOps.scala | 5 +-
.../sql/types/ops/TimestampNanosTypeApiOps.scala | 13 ++++
.../apache/spark/sql/types/ops/TypeApiOps.scala | 28 ++++++--
.../scala/org/apache/spark/sql/RowJsonSuite.scala | 79 +++++++++++++++++++++-
5 files changed, 128 insertions(+), 12 deletions(-)
diff --git a/sql/api/src/main/scala/org/apache/spark/sql/Row.scala
b/sql/api/src/main/scala/org/apache/spark/sql/Row.scala
index 3a0e4d45f937..b6b5c7da9644 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/Row.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/Row.scala
@@ -18,7 +18,7 @@
package org.apache.spark.sql
import java.sql.{Date, Timestamp}
-import java.time.{Instant, LocalDate, LocalDateTime}
+import java.time.{Instant, LocalDate, LocalDateTime, LocalTime}
import java.util.Base64
import scala.collection.mutable
@@ -632,8 +632,14 @@ trait Row extends Serializable {
if (value == null) {
JNull
} else {
+ // A public Row holds external values (e.g. java.time.LocalTime for
TimeType), so render
+ // through the framework's formatExternal rather than format, which
expects the internal
+ // representation and would otherwise fail the value cast. A framework
type either returns a
+ // rendered string or raises its own error (e.g. the nanosecond
timestamp types raise the
+ // unsupported-rendering error); types outside the framework fall back
to legacy rendering.
TypeApiOps(dataType)
- .map(ops => JString(ops.format(value)))
+ .flatMap(_.formatExternal(value))
+ .map(JString(_))
.getOrElse(toJsonDefault(value, dataType))
}
@@ -642,6 +648,11 @@ trait Row extends Serializable {
case (b: Byte, _) => JLong(b)
case (s: Short, _) => JLong(s)
case (i: Int, _) => JLong(i)
+ // A public Row holds the external java.time.LocalTime for a TimeType
column (SPARK-54451).
+ // With the Types Framework off, TypeApiOps returns None and we land
here, so render the
+ // LocalTime directly, mirroring HiveResult's legacy fallback. The Long
case below is the
+ // internal representation that a public Row never holds; it stays for
non-Row callers.
+ case (lt: LocalTime, _: TimeType) => JString(timeFormatter.format(lt))
case (nanos: Long, _: TimeType) => JString(timeFormatter.format(nanos))
case (l: Long, _) => JLong(l)
case (f: Float, _) => JDouble(f)
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimeTypeApiOps.scala
b/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimeTypeApiOps.scala
index dd8f0398aba9..7b5596109290 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimeTypeApiOps.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimeTypeApiOps.scala
@@ -24,7 +24,7 @@ import org.apache.arrow.vector.types.pojo.ArrowType
import org.apache.spark.sql.catalyst.encoders.AgnosticEncoder
import org.apache.spark.sql.catalyst.encoders.AgnosticEncoders.LocalTimeEncoder
-import org.apache.spark.sql.catalyst.util.{FractionTimeFormatter,
SparkDateTimeUtils, TimeFormatter}
+import org.apache.spark.sql.catalyst.util.{FractionTimeFormatter,
TimeFormatter}
import org.apache.spark.sql.types.{DataType, TimeType}
/**
@@ -82,8 +82,7 @@ class TimeTypeApiOps(val t: TimeType) extends TypeApiOps {
})
override def formatExternal(value: Any): Option[String] = {
- val nanos =
SparkDateTimeUtils.localTimeToNanos(value.asInstanceOf[LocalTime])
- Some(timeFormatter.format(nanos))
+ Some(timeFormatter.format(value.asInstanceOf[LocalTime]))
}
override def thriftTypeName: Option[String] = Some("STRING_TYPE")
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimestampNanosTypeApiOps.scala
b/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimestampNanosTypeApiOps.scala
index ee6125c8f1af..fc3bb947ee8f 100644
---
a/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimestampNanosTypeApiOps.scala
+++
b/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimestampNanosTypeApiOps.scala
@@ -57,6 +57,19 @@ abstract class TimestampNanosTypeApiOps extends TypeApiOps
with DataTypeErrorsBa
override def format(v: Any): String =
throw DataTypeErrors.cannotConvertNanosTimestampToStringError(dataType)
+ // Row JSON (Row.json / Row.prettyJson) holds the external Row value
(java.time.Instant for LTZ,
+ // java.time.LocalDateTime for NTZ), but rendering nanosecond timestamps
from this zone-less path
+ // is not supported yet (same reason as format above), so raise the
user-facing
+ // UNSUPPORTED_FEATURE.TIMESTAMP_NANOS_TO_STRING error here rather than
silently truncating.
+ override def formatExternal(value: Any): Option[String] =
+ throw DataTypeErrors.cannotConvertNanosTimestampToStringError(dataType)
+
+ // The Hive result path (HiveResult.toHiveString) is zone-aware and renders
nanosecond timestamps
+ // through its own default formatter, so return None here to fall through to
it rather than raise
+ // the unsupported-rendering error that the zone-less single-arg
formatExternal above throws. This
+ // is a temporary override until nanos external rendering is unified across
the two paths.
+ override def formatExternal(value: Any, nested: Boolean): Option[String] =
None
+
override def toSQLValue(v: Any): String = s"$sqlTypeName '${format(v)}'"
// ==================== Row Encoding ====================
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TypeApiOps.scala
b/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TypeApiOps.scala
index ae690119f148..40a158d70af3 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TypeApiOps.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/types/ops/TypeApiOps.scala
@@ -118,15 +118,35 @@ trait TypeApiOps extends Serializable {
/** Creates a converter function for Python/Py4J interop. */
def makeFromJava: Option[Any => Any] = None
- // ==================== Hive Formatting (optional) ====================
+ // ==================== External-Value Formatting (optional)
====================
/**
- * Formats an external-type value for Hive output. Most types override this
simple version.
- * Types that need different formatting when nested should override the
2-param overload.
+ * Renders an external (public-facing) value as a display string, or returns
None to let the
+ * caller fall back to its own legacy rendering. Unlike [[format]], which
takes the internal
+ * representation, this takes the external value a public Row holds (e.g.
java.time.LocalTime
+ * for TimeType).
+ *
+ * Consumer: Row JSON (Row.json / Row.prettyJson). Semantics:
+ * - Some(s): s is used as the rendered JSON string.
+ * - None: Row JSON falls back to its legacy toJsonDefault rendering.
+ * - throw: an implementation may raise instead, to signal that rendering
this type on this
+ * zone-less path is unsupported (e.g. the nanosecond timestamp types
raise
+ * UNSUPPORTED_FEATURE.TIMESTAMP_NANOS_TO_STRING).
+ *
+ * Returning None silently routes the type into the legacy path, so a type
that must be rendered
+ * by the framework should override this (every currently registered type
does).
*/
def formatExternal(value: Any): Option[String] = None
- /** Formats with nesting context. Default delegates to the simple version. */
+ /**
+ * Renders an external value for Hive-style output
(HiveResult.toHiveString). `nested` indicates
+ * whether the value appears inside an array/map/struct, which may
format/quote it differently.
+ * Semantics mirror the single-arg overload: Some(s) is used directly, None
falls back to
+ * HiveResult's zone-aware legacy rendering. The default delegates to the
single-arg overload;
+ * override it separately when the two consumers need different behavior
(e.g. the nanosecond
+ * timestamp types throw on the zone-less Row JSON path but return None here
so the zone-aware
+ * Hive path renders them).
+ */
def formatExternal(value: Any, nested: Boolean): Option[String] =
formatExternal(value)
// ==================== Thrift Mapping (optional) ====================
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/RowJsonSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/RowJsonSuite.scala
index 49138532616e..edc7b9512d38 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/RowJsonSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/RowJsonSuite.scala
@@ -17,20 +17,22 @@
package org.apache.spark.sql
import java.sql.{Date, Timestamp}
-import java.time.{LocalDate, LocalDateTime}
+import java.time.{LocalDate, LocalDateTime, LocalTime, ZoneOffset}
import org.json4s.JsonAST.{JArray, JBool, JDecimal, JDouble, JLong, JNull,
JObject, JString, JValue}
-import org.apache.spark.{SparkFunSuite, SparkIllegalArgumentException}
+import org.apache.spark.{SparkFunSuite, SparkIllegalArgumentException,
SparkUnsupportedOperationException}
import org.apache.spark.sql.catalyst.encoders.{ExamplePoint, ExamplePointUDT}
import org.apache.spark.sql.catalyst.expressions.GenericRowWithSchema
+import org.apache.spark.sql.catalyst.plans.SQLHelper
import org.apache.spark.sql.errors.DataTypeErrors.{toSQLType, toSQLValue}
+import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
/**
* Test suite for [[Row]] JSON serialization.
*/
-class RowJsonSuite extends SparkFunSuite {
+class RowJsonSuite extends SparkFunSuite with SQLHelper {
private val schema = new StructType()
.add("c1", "string")
.add("c2", IntegerType)
@@ -122,6 +124,77 @@ class RowJsonSuite extends SparkFunSuite {
new ExamplePointUDT,
JArray(JDouble(3.4d) :: JDouble(8.98d) :: Nil))
+ // A public Row holds external values, so a TimeType column carries a
java.time.LocalTime, not the
+ // internal Long nanos-of-day. Row JSON must render it via the framework's
formatExternal; before
+ // this fix Row.toJson called the internal-value format, which cast
LocalTime to Long and threw
+ // ClassCastException for any TIME column.
+ private def timeRowJson(value: LocalTime, precision: Int): JValue =
+ new GenericRowWithSchema(Array(value), new StructType().add("a",
TimeType(precision))).jsonValue
+
+ test("SPARK-57338: TIME column renders the external LocalTime in JSON") {
+ withSQLConf(SQLConf.TYPES_FRAMEWORK_ENABLED.key -> "true") {
+ assert(timeRowJson(LocalTime.of(12, 13, 14), TimeType.MICROS_PRECISION)
===
+ JObject("a" -> JString("12:13:14")))
+ // The fraction is rendered up to microsecond resolution with trailing
zeros trimmed.
+ assert(timeRowJson(LocalTime.of(1, 2, 3, 123456000),
TimeType.MICROS_PRECISION) ===
+ JObject("a" -> JString("01:02:03.123456")))
+ assert(timeRowJson(LocalTime.of(10, 30, 0, 100000000),
TimeType.MICROS_PRECISION) ===
+ JObject("a" -> JString("10:30:00.1")))
+ assert(timeRowJson(LocalTime.MIDNIGHT, TimeType.MICROS_PRECISION) ===
+ JObject("a" -> JString("00:00:00")))
+ }
+ }
+
+ test("SPARK-57338: TIME column JSON rendering is independent of the column
precision") {
+ withSQLConf(SQLConf.TYPES_FRAMEWORK_ENABLED.key -> "true") {
+ val time = LocalTime.of(1, 2, 3, 123456000)
+ (TimeType.MIN_PRECISION to TimeType.MAX_PRECISION).foreach { p =>
+ assert(timeRowJson(time, p) === JObject("a" ->
JString("01:02:03.123456")))
+ }
+ }
+ }
+
+ test("SPARK-57338: null TIME column renders as JSON null") {
+ withSQLConf(SQLConf.TYPES_FRAMEWORK_ENABLED.key -> "true") {
+ assert(timeRowJson(null, TimeType.MICROS_PRECISION) === JObject("a" ->
JNull))
+ }
+ }
+
+ // With the Types Framework off (the production default), TypeApiOps returns
None and Row JSON
+ // falls back to the legacy toJsonDefault, which must still render the
external LocalTime that a
+ // public Row holds for a TIME column - matching HiveResult's legacy
fallback - rather than
+ // failing with FAILED_ROW_TO_JSON.
+ test("SPARK-57338: TIME column renders via the legacy path with the
framework disabled") {
+ withSQLConf(SQLConf.TYPES_FRAMEWORK_ENABLED.key -> "false") {
+ assert(timeRowJson(LocalTime.of(12, 13, 14), TimeType.MICROS_PRECISION)
===
+ JObject("a" -> JString("12:13:14")))
+ assert(timeRowJson(LocalTime.of(1, 2, 3, 123456000),
TimeType.MICROS_PRECISION) ===
+ JObject("a" -> JString("01:02:03.123456")))
+ assert(timeRowJson(null, TimeType.MICROS_PRECISION) === JObject("a" ->
JNull))
+ }
+ }
+
+ // Routing the external value through formatExternal must not silently
change the nanosecond
+ // timestamp behavior: those ops raise the clean unsupported-rendering error
directly from
+ // formatExternal instead of mis-rendering the external value as a
microsecond timestamp.
+ test("SPARK-57338: nanosecond timestamp column raises the
unsupported-rendering error in JSON") {
+ withSQLConf(
+ SQLConf.TYPES_FRAMEWORK_ENABLED.key -> "true",
+ SQLConf.TIMESTAMP_NANOS_TYPES_ENABLED.key -> "true") {
+ val ldt = LocalDateTime.of(2020, 1, 1, 12, 0, 0, 123456789)
+ Seq[(Any, DataType)](
+ ldt -> TimestampNTZNanosType(9),
+ ldt.toInstant(ZoneOffset.UTC) -> TimestampLTZNanosType(9)).foreach {
case (value, dt) =>
+ checkError(
+ exception = intercept[SparkUnsupportedOperationException] {
+ new GenericRowWithSchema(Array(value), new StructType().add("a",
dt)).jsonValue
+ },
+ condition = "UNSUPPORTED_FEATURE.TIMESTAMP_NANOS_TO_STRING",
+ parameters = Map("dataType" -> toSQLType(dt)))
+ }
+ }
+ }
+
test("no schema") {
val e = intercept[IllegalArgumentException] {
Row("a").jsonValue
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
