You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "MaxGekk (via GitHub)" <gi...@apache.org> on 2026/06/17 06:29:56 UTC
[PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
MaxGekk opened a new pull request, #56557:
URL: https://github.com/apache/spark/pull/56557
### What changes were proposed in this pull request?
This PR adds end-to-end support for nanosecond-capable timestamp types in ORC for both datasource v1 and v2, including Hive ORC paths.
It introduces ORC schema/type mappings for nanos timestamps, implements native ORC serializer/deserializer and vectorized reader conversions for `TimestampNTZNanosType` and `TimestampLTZNanosType`, and removes ORC-specific datasource guardrails that previously rejected nanos timestamp types.
For Hive ORC, it updates serializer/deserializer and inspector/table-reader conversion logic so nanos timestamps round-trip correctly while remaining compatible with Hive type parsing.
It also extends tests to cover:
- ORC support in v1 and v2 for precisions 7/8/9
- Native ORC vectorized vs non-vectorized read parity
- Hive ORC nanos support with `CONVERT_METASTORE_ORC` enabled/disabled
- Backward compatibility: plain ORC `timestamp` (without Spark nanos metadata) is still inferred as `TimestampType`
### Why are the changes needed?
SPARK-57455 tracks enabling ORC to support Spark nanosecond timestamp types. ORC was still rejecting these types in datasource capability checks and lacked complete conversion coverage in native/Hive read-write paths. Without these changes, nanos timestamp columns could not be reliably written/read through ORC.
### Does this PR introduce _any_ user-facing change?
Yes.
Users can now write and read `TimestampNTZNanosType(p)` and `TimestampLTZNanosType(p)` (`p` in 7..9) with ORC in both datasource v1 and v2, including Hive ORC integration paths.
### How was this patch tested?
- `./dev/scalastyle`
- `./build/mvn scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl sql/api -pl sql/connect/common -pl sql/connect/server -pl sql/connect/shims -pl sql/connect/client/jvm`
- `build/sbt 'sql/Test/compile'`
- `build/sbt 'hive/Test/compile'`
- `build/sbt 'sql/testOnly org.apache.spark.sql.FileBasedDataSourceSuite -- -z "SPARK-57166"'`
- `build/sbt 'hive/testOnly org.apache.spark.sql.hive.orc.HiveOrcSourceSuite -- -z "SPARK-57166"'`
- `build/sbt 'sql/testOnly org.apache.spark.sql.execution.datasources.orc.OrcV1QuerySuite -- -z "SPARK-57455"'`
- `build/sbt 'sql/testOnly org.apache.spark.sql.execution.datasources.orc.OrcV2QuerySuite -- -z "SPARK-57455"'`
- `build/sbt 'sql/testOnly org.apache.spark.sql.execution.datasources.orc.OrcSourceV1Suite -- -z "SPARK-57455"'`
- `build/sbt 'sql/testOnly org.apache.spark.sql.execution.datasources.orc.OrcSourceV2Suite -- -z "SPARK-57455"'`
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor Codex 5.3
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3431857451
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcUtils.scala:
##########
@@ -196,13 +196,34 @@ object OrcUtils extends Logging {
requiredSchema: StructType,
orcSchema: TypeDescription,
conf: Configuration): Option[(Array[Int], Boolean)] = {
+ def isOrcTimestamp(dt: DataType): Boolean = dt match {
+ case TimestampType | TimestampNTZType | _: AnyTimestampNanoType => true
+ case _ => false
+ }
+
+ // The ORC reader does not coerce between timestamp families/precisions, except between
+ // nanos timestamps of the same kind (NTZ or LTZ), which share an ORC physical category and
+ // only differ by the precision applied on read. Any other mismatch (zone or micros<->nanos)
+ // would otherwise fail obscurely, so reject it with a clear error.
+ def timestampReadCompatible(orcType: DataType, dataType: DataType): Boolean =
+ (orcType, dataType) match {
+ case _ if orcType == dataType => true
+ case (_: TimestampNTZNanosType, _: TimestampNTZNanosType) => true
+ case (_: TimestampLTZNanosType, _: TimestampLTZNanosType) => true
+ case _ => false
+ }
+
def checkTimestampCompatibility(orcCatalystSchema: StructType, dataSchema: StructType): Unit = {
orcCatalystSchema.fields.map(_.dataType).zip(dataSchema.fields.map(_.dataType)).foreach {
case (TimestampType, TimestampNTZType) =>
throw QueryExecutionErrors.cannotConvertOrcTimestampToTimestampNTZError()
case (TimestampNTZType, TimestampType) =>
throw QueryExecutionErrors.cannotConvertOrcTimestampNTZToTimestampLTZError()
case (t1: StructType, t2: StructType) => checkTimestampCompatibility(t1, t2)
Review Comment:
One question regarding pre-existing behaviour: why do we not recurse into ArrayType/MapType too? Does the checker look inside containers where timestamps are nested inside array or map?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3431845606
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/TableReader.scala:
##########
@@ -488,7 +489,18 @@ private[hive] object HadoopTableReader extends HiveInspectors with Logging {
row.update(ordinal, HiveShim.toCatalystDecimal(oi, value))
case oi: TimestampObjectInspector =>
(value: Any, row: InternalRow, ordinal: Int) =>
- row.setLong(ordinal, DateTimeUtils.fromJavaTimestamp(oi.getPrimitiveJavaObject(value)))
+ attr.dataType match {
Review Comment:
TableReader.scala duplicates the top-level TimestampObjectInspector nanos logic rather than using unwrapperFor(oi, dataType).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56557:
URL: https://github.com/apache/spark/pull/56557#issuecomment-4741396512
All tests passed: https://github.com/MaxGekk/spark/runs/82066403399.
Merging to master/4.x. Thank you, @HyukjinKwon and @uros-b for review.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3431522112
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -752,6 +797,73 @@ private[hive] trait HiveInspectors {
}
}
+ /**
+ * Returns an unwrapper that converts a Hive value into a Catalyst value, using the target
+ * Catalyst `dataType` to preserve nanosecond timestamp precision. The plain
+ * `unwrapperFor(ObjectInspector)` cannot do this because a Hive `TimestampObjectInspector`
+ * maps to micros by default; here the nanos timestamp types are produced as `TimestampNanosVal`,
+ * recursing through array/map/struct so nested nanos timestamps round-trip correctly. Any other
+ * type is delegated to the `ObjectInspector`-only overload.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector, dataType: DataType): Any => Any =
+ (objectInspector, dataType) match {
+ case (ti: TimestampObjectInspector, t: TimestampNTZNanosType) =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.localDateTimeToTimestampNanos(
+ ti.getPrimitiveJavaObject(data).toLocalDateTime, t.precision)
+ } else {
+ null
+ }
+ }
+ case (ti: TimestampObjectInspector, t: TimestampLTZNanosType) =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.instantToTimestampNanos(
+ ti.getPrimitiveJavaObject(data).toInstant, t.precision)
+ } else {
+ null
+ }
+ }
+ case (li: ListObjectInspector, ArrayType(elementType, _)) =>
Review Comment:
Small duplication in Hive OrcFileFormat unwrappers: Top-level TimestampObjectInspector fields get inline nanos handling, while nested fields use unwrapperFor(oi, dataType). Functionally fine, but you could simplify to always call unwrapperFor(fieldRef.getFieldObjectInspector, field.dataType) and drop the duplicated top-level branch.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56557:
URL: https://github.com/apache/spark/pull/56557#issuecomment-4731720635
@uros-b Added nested/complex-type coverage in c15a492d3ea for ORC nanos timestamps on the native path (`struct`, `array`, `map`) across precisions 7/8/9 and vectorized/non-vectorized readers.\n\nI also deduplicated the helper by moving `nanosTimestampDf` into `OrcTest` and reusing it from both suites.\n\nI tried adding equivalent Hive serde nested coverage, but that currently fails with `ClassCastException` (`Long` vs `TimestampNanosVal`) in nested nanos deserialization, so I kept Hive coverage focused on the supported flat-path test already in this PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56557:
URL: https://github.com/apache/spark/pull/56557#issuecomment-4732122017
@uros-b Update: the Hive serde nested nanos limitation I mentioned earlier is now fixed in 80583a20dea.
Root cause: `HiveInspectors.unwrapperFor(ObjectInspector)` had no Catalyst type context, so nanos timestamps nested inside `struct`/`array`/`map` were deserialized as micros `Long` instead of `TimestampNanosVal` (the nanos special-casing only covered top-level columns), causing a `ClassCastException`.
Fix: added a data-type-aware `unwrapperFor(ObjectInspector, DataType)` that produces `TimestampNanosVal` for nanos timestamp types and recurses through array/map/struct, used from `OrcFileFormat` for nested fields containing a nanos timestamp. The Hive nested/complex test is restored and expanded to cover `struct`/`array`/`map` across precisions 7/8/9.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56557:
URL: https://github.com/apache/spark/pull/56557#issuecomment-4730039777
@dongjoon-hyun @HyukjinKwon @uros-b Could you look at the PR when you have time.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3428193942
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcQuerySuite.scala:
##########
@@ -917,6 +918,130 @@ abstract class OrcQuerySuite extends OrcQueryTest with SharedSparkSession {
}
}
+ // Builds a single-column ("ts") DataFrame from external java.time values, letting the schema
+ // precision truncate the sub-microsecond digits: an NTZ column takes java.time.LocalDateTime
+ // values, an LTZ column takes the same wall clocks as java.time.Instant at UTC.
+ private def nanosTimestampDf(nanosType: DataType, wallClocks: Seq[LocalDateTime]): DataFrame = {
Review Comment:
There's some duplication with HiveOrcSourceSuite. Can we use a common helper?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3431826121
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -752,6 +797,73 @@ private[hive] trait HiveInspectors {
}
}
+ /**
+ * Returns an unwrapper that converts a Hive value into a Catalyst value, using the target
+ * Catalyst `dataType` to preserve nanosecond timestamp precision. The plain
+ * `unwrapperFor(ObjectInspector)` cannot do this because a Hive `TimestampObjectInspector`
+ * maps to micros by default; here the nanos timestamp types are produced as `TimestampNanosVal`,
+ * recursing through array/map/struct so nested nanos timestamps round-trip correctly. Any other
+ * type is delegated to the `ObjectInspector`-only overload.
+ */
+ def unwrapperFor(objectInspector: ObjectInspector, dataType: DataType): Any => Any =
+ (objectInspector, dataType) match {
+ case (ti: TimestampObjectInspector, t: TimestampNTZNanosType) =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.localDateTimeToTimestampNanos(
+ ti.getPrimitiveJavaObject(data).toLocalDateTime, t.precision)
+ } else {
+ null
+ }
+ }
+ case (ti: TimestampObjectInspector, t: TimestampLTZNanosType) =>
+ data: Any => {
+ if (data != null) {
+ DateTimeUtils.instantToTimestampNanos(
+ ti.getPrimitiveJavaObject(data).toInstant, t.precision)
+ } else {
+ null
+ }
+ }
+ case (li: ListObjectInspector, ArrayType(elementType, _)) =>
Review Comment:
Done in 8c7a4640242. Dropped the inline top-level `TimestampObjectInspector` branch; all nanos timestamps (top-level and nested) now go through `unwrapperFor(oi, dataType)`, while non-nanos fields keep the `HiveStructField` fast paths (`setInt`/`setLong`/...), so this avoids regressing the primitive paths while removing the duplication.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcUtils.scala:
##########
@@ -291,6 +291,8 @@ object OrcUtils extends Logging {
case m: MapType =>
s"map<${getOrcSchemaString(m.keyType)},${getOrcSchemaString(m.valueType)}>"
case _: DayTimeIntervalType | _: TimestampNTZType | _: TimeType => LongType.catalogString
+ case _: TimestampLTZNanosType => "timestamp with local time zone"
Review Comment:
Good catch — addressed in 8c7a4640242. `checkTimestampCompatibility` now also rejects nanos-involved mismatches (cross-zone, or micros<->nanos) with a clear `UNSUPPORTED_FEATURE.ORC_TYPE_CAST` error; nanos of the same kind (NTZ/LTZ) still coerce by precision since they share an ORC physical category. Added a `QueryExecutionErrorsSuite` case reading a `timestamp_ntz(9)` file against a micros `TIMESTAMP_NTZ` schema.
##########
sql/core/src/test/scala/org/apache/spark/sql/FileBasedDataSourceSuite.scala:
##########
@@ -1394,6 +1395,31 @@ class FileBasedDataSourceSuite extends SharedSparkSession
}
}
}
+
+ test("SPARK-57166: ORC supports nanosecond timestamp types in v1 and v2") {
+ withSQLConf(SQLConf.TIMESTAMP_NANOS_TYPES_ENABLED.key -> "true") {
+ // Validate both v1 and v2 ORC paths.
+ Seq(true, false).foreach { useV1 =>
+ val useV1List = if (useV1) "orc" else ""
+ withSQLConf(SQLConf.USE_V1_SOURCE_LIST.key -> useV1List) {
+ foreachNanosPrecision { precision =>
+ Seq(TimestampNTZNanosType(precision), TimestampLTZNanosType(precision)).foreach {
+ nanosType =>
+ withTempDir { dir =>
+ val nanosLiteral = Literal.create(new TimestampNanosVal(0L, 0.toShort), nanosType)
Review Comment:
Aligned in 8c7a4640242 — the positive ORC test now builds rows from external `java.time` values (matching `nanosTimestampDf`) instead of `TimestampNanosVal` + `Literal`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3433491242
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/TableReader.scala:
##########
@@ -488,7 +489,18 @@ private[hive] object HadoopTableReader extends HiveInspectors with Logging {
row.update(ordinal, HiveShim.toCatalystDecimal(oi, value))
case oi: TimestampObjectInspector =>
(value: Any, row: InternalRow, ordinal: Int) =>
- row.setLong(ordinal, DateTimeUtils.fromJavaTimestamp(oi.getPrimitiveJavaObject(value)))
+ attr.dataType match {
Review Comment:
Done in 1e2004ffb18. Dropped the inline nanos branch in `TableReader` and route nanos timestamps through `unwrapperFor(oi, dataType)` (same as `OrcFileFormat`); micros timestamps keep the `setLong` fast path.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcUtils.scala:
##########
@@ -196,13 +196,34 @@ object OrcUtils extends Logging {
requiredSchema: StructType,
orcSchema: TypeDescription,
conf: Configuration): Option[(Array[Int], Boolean)] = {
+ def isOrcTimestamp(dt: DataType): Boolean = dt match {
+ case TimestampType | TimestampNTZType | _: AnyTimestampNanoType => true
+ case _ => false
+ }
+
+ // The ORC reader does not coerce between timestamp families/precisions, except between
+ // nanos timestamps of the same kind (NTZ or LTZ), which share an ORC physical category and
+ // only differ by the precision applied on read. Any other mismatch (zone or micros<->nanos)
+ // would otherwise fail obscurely, so reject it with a clear error.
+ def timestampReadCompatible(orcType: DataType, dataType: DataType): Boolean =
+ (orcType, dataType) match {
+ case _ if orcType == dataType => true
+ case (_: TimestampNTZNanosType, _: TimestampNTZNanosType) => true
+ case (_: TimestampLTZNanosType, _: TimestampLTZNanosType) => true
+ case _ => false
+ }
+
def checkTimestampCompatibility(orcCatalystSchema: StructType, dataSchema: StructType): Unit = {
orcCatalystSchema.fields.map(_.dataType).zip(dataSchema.fields.map(_.dataType)).foreach {
case (TimestampType, TimestampNTZType) =>
throw QueryExecutionErrors.cannotConvertOrcTimestampToTimestampNTZError()
case (TimestampNTZType, TimestampType) =>
throw QueryExecutionErrors.cannotConvertOrcTimestampNTZToTimestampLTZError()
case (t1: StructType, t2: StructType) => checkTimestampCompatibility(t1, t2)
Review Comment:
Good question — it was pre-existing (struct-only). Fixed in 1e2004ffb18: `checkTimestampCompatibility` now recurses into `ArrayType`/`MapType` as well, so a timestamp mismatch nested inside an array/map (e.g. `array<timestamp_ntz(9)>` read as `array<timestamp_ntz>`) also gets the clear `UNSUPPORTED_FEATURE.ORC_TYPE_CAST` error. Added a `QueryExecutionErrorsSuite` case for the array-nested mismatch.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3431531265
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcUtils.scala:
##########
@@ -291,6 +291,8 @@ object OrcUtils extends Logging {
case m: MapType =>
s"map<${getOrcSchemaString(m.keyType)},${getOrcSchemaString(m.valueType)}>"
case _: DayTimeIntervalType | _: TimestampNTZType | _: TimeType => LongType.catalogString
+ case _: TimestampLTZNanosType => "timestamp with local time zone"
Review Comment:
Note: checkTimestampCompatibility doesn’t know about nanos; OrcUtils.requestedColumnIds still only guards micros TimestampType ↔ TimestampNTZType mismatches. A file with timestamp_ntz(9) metadata read against a micros TimestampNTZType schema (or the reverse) won’t get a clear error. Do you intend to tackle this in the current PR, or would you prefer to do it separately?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3429069343
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcQuerySuite.scala:
##########
@@ -917,6 +918,130 @@ abstract class OrcQuerySuite extends OrcQueryTest with SharedSparkSession {
}
}
+ // Builds a single-column ("ts") DataFrame from external java.time values, letting the schema
+ // precision truncate the sub-microsecond digits: an NTZ column takes java.time.LocalDateTime
+ // values, an LTZ column takes the same wall clocks as java.time.Instant at UTC.
+ private def nanosTimestampDf(nanosType: DataType, wallClocks: Seq[LocalDateTime]): DataFrame = {
Review Comment:
Good point, done in c15a492d3ea. I moved `nanosTimestampDf` into `OrcTest` so both `OrcQuerySuite` and `HiveOrcSourceSuite` share the same helper and removed the duplicated local methods.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #56557:
URL: https://github.com/apache/spark/pull/56557#discussion_r3431535211
##########
sql/core/src/test/scala/org/apache/spark/sql/FileBasedDataSourceSuite.scala:
##########
@@ -1394,6 +1395,31 @@ class FileBasedDataSourceSuite extends SharedSparkSession
}
}
}
+
+ test("SPARK-57166: ORC supports nanosecond timestamp types in v1 and v2") {
+ withSQLConf(SQLConf.TIMESTAMP_NANOS_TYPES_ENABLED.key -> "true") {
+ // Validate both v1 and v2 ORC paths.
+ Seq(true, false).foreach { useV1 =>
+ val useV1List = if (useV1) "orc" else ""
+ withSQLConf(SQLConf.USE_V1_SOURCE_LIST.key -> useV1List) {
+ foreachNanosPrecision { precision =>
+ Seq(TimestampNTZNanosType(precision), TimestampLTZNanosType(precision)).foreach {
+ nanosType =>
+ withTempDir { dir =>
+ val nanosLiteral = Literal.create(new TimestampNanosVal(0L, 0.toShort), nanosType)
Review Comment:
Nit: FileBasedDataSourceSuite’s new positive ORC test still builds rows via TimestampNanosVal + Literal, while OrcQuerySuite / HiveOrcSourceSuite use external java.time via nanosTimestampDf. Harmless, but aligning them would be slightly cleaner.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #56557:
URL: https://github.com/apache/spark/pull/56557#issuecomment-4730270338
Please add tests for nested/complex type handling.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57455][SQL] Support nanosecond timestamp types in ORC [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #56557: [SPARK-57455][SQL] Support nanosecond timestamp types in ORC
URL: https://github.com/apache/spark/pull/56557
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org