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/05/27 21:35:28 UTC
[PR] [WIP][SPARK-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
MaxGekk opened a new pull request, #56158:
URL: https://github.com/apache/spark/pull/56158
### What changes were proposed in this pull request?
This PR wires `java.time.LocalDateTime` / `java.time.Instant` through the encoder, converter and Dataset stack for the nanosecond-capable timestamp types `TimestampNTZNanosType(p)` and `TimestampLTZNanosType(p)` introduced by SPARK-56981, so that `spark.createDataFrame(rows, schema).collect()` over a schema declaring a nanos timestamp column preserves full nanosecond precision end-to-end.
Concretely:
- `SparkDateTimeUtils` (mixed into `DateTimeUtils`): four new helpers
- `localDateTimeToTimestampNanos(LocalDateTime): TimestampNanosVal`
- `timestampNanosToLocalDateTime(TimestampNanosVal): LocalDateTime`
- `instantToTimestampNanos(Instant): TimestampNanosVal`
- `timestampNanosToInstant(TimestampNanosVal): Instant`
Sub-microsecond digits (`getNano % 1000`) are kept as `nanosWithinMicro` in `[0, 999]`; the integral micro part reuses the existing `instantToMicros` / `localDateTimeToMicros` helpers (including the `MIN_SECONDS` overflow guard).
- `CatalystTypeConverters`: new `TimestampNTZNanosConverter` and `TimestampLTZNanosConverter`, dispatched from `getConverterForTypeDefault`. The catalyst-side value is `TimestampNanosVal` and `toScalaImpl` reads via `row.getTimestampNTZNanos` / `row.getTimestampLTZNanos`. `TimestampLTZNanosType` always maps to `Instant`, independent of `spark.sql.datetime.java8API.enabled` (legacy `java.sql.Timestamp` is intentionally out of scope here).
- `AgnosticEncoders`: two new precision-aware leaf encoders
- `LocalDateTimeNanosEncoder(precision)` -> `TimestampNTZNanosType(precision)`
- `InstantNanosEncoder(precision)` -> `TimestampLTZNanosType(precision)`
- `RowEncoder.encoderForDataType`: maps the new nanos data types to the new encoders.
- `SerializerBuildHelper` / `DeserializerBuildHelper`: new `StaticInvoke` builders and dispatch cases that route the new encoders through `DateTimeUtils.{localDateTimeToTimestampNanos, instantToTimestampNanos, timestampNanosToLocalDateTime, timestampNanosToInstant}`.
- `EncoderUtils.dataTypeJavaClassDefault` / `javaBoxedType`: add `TimestampNTZNanosType` / `TimestampLTZNanosType` -> `classOf[TimestampNanosVal]`, so the `StaticInvoke` codegen picks up the correct Java class when the path is bound to a nanos column.
`Encoders.LOCALDATETIME()` / `Encoders.INSTANT()`, the schema-less `convertToCatalyst` fallback, cast / precision-truncation behavior, an SQL `Timestamp` external type for LTZ Nanos, and ordering / hash for the physical nanos types are intentionally left out of this PR and tracked separately.
### Why are the changes needed?
`SPARK-56981` added physical row storage for nanosecond timestamps, but there was no conversion layer from the external Java types. As a result, even with the nanos schema explicitly declared, Dataset create -> internal row -> collect roundtrips silently truncated to micros:
```scala
val ldt = java.time.LocalDateTime.parse("2019-02-26T16:56:00.123456789")
val schema = new StructType().add("t", TimestampNTZNanosType(9))
spark.createDataFrame(java.util.Arrays.asList(Row(ldt)), schema).collect()(0).get(0)
// before this PR: 2019-02-26T16:56:00.123456 (last 3 digits dropped)
// after this PR: 2019-02-26T16:56:00.123456789 (preserved)
```
This change is required for the rest of the nanos-timestamp SPIP (SPARK-56822) to be useful from the DataFrame / Dataset API.
### Does this PR introduce _any_ user-facing change?
Yes, but only for the new `TimestampNTZNanosType` / `TimestampLTZNanosType` (preview feature, gated by `spark.sql.timestampNanosTypes.enabled`):
- A `Dataset[Row]` whose schema declares a nanos timestamp column now accepts `LocalDateTime` (NTZ Nanos) / `Instant` (LTZ Nanos) and returns them on `collect` with full nanosecond precision.
- A bare `LocalDateTime` or `Instant` value (without an explicit schema) still resolves to the existing micro `TimestampNTZType` / `TimestampType` encoders, so `Dataset[LocalDateTime]` / `Dataset[Instant]` behavior is unchanged.
Existing micro timestamp behavior is unchanged.
### How was this patch tested?
New unit tests, all passing locally:
- `DateTimeUtilsSuite` - three `SPARK-57033` tests for the helpers (LDT, Instant, randomized roundtrip across the full valid range, including pre-epoch, epoch, max range, sub-micro digits, and the `nanosWithinMicro in [0, 999]` invariant).
- `CatalystTypeConvertersSuite` - both directions for `TimestampNTZNanosType` / `TimestampLTZNanosType` across precisions 7-9, a flag-independence test for the LTZ Nanos path, and a null-row roundtrip via `Row` + schema.
- `RowEncoderSuite` - encode/decode tests for both nanos types across all precisions and both codegen / interpreted paths, plus a Java8 flag-independence test.
- `DatasetSuite` - end-to-end `spark.createDataFrame(rows, schema).collect()` with sub-micro fractional digits, a max-range edge value, and a null row.
- `JavaDatasetSuite` - `testTimestampNanosRowEncoder` exercising `spark.createDataset(rows, Encoders.row(schema))` from Java.
Regression check across adjacent suites (no failures):
- `build/sbt 'catalyst/testOnly *DateTimeUtilsSuite *CatalystTypeConvertersSuite *RowEncoderSuite'` - 233/233 pass.
- `build/sbt 'catalyst/testOnly *ExpressionEncoderSuite *TimestampNanosRowSuite *LiteralExpressionSuite'` - 453/453 pass.
- `build/sbt 'sql/testOnly *DatasetSuite'` - 276/276 pass (221 Scala + 55 Java).
- `./dev/scalastyle` and `./dev/lint-java` clean.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor 2.0 (Claude Opus 4.7)
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3330632359
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -97,6 +99,14 @@ object RowEncoder extends DataTypeErrorsBase {
case TimestampType if SqlApiConf.get.datetimeJava8ApiEnabled => InstantEncoder(lenient)
case TimestampType => TimestampEncoder(lenient)
case TimestampNTZType => LocalDateTimeEncoder
+ // Nano timestamp types intentionally do not honor `lenient`: legacy `java.sql.Timestamp` /
+ // `java.sql.Date` external types are out of scope for nanosecond precision (SPARK-57033).
+ case t: TimestampNTZNanosType =>
Review Comment:
Yes - this is intentional. The scope of SPARK-57033 is the classic-mode
java.time <-> internal-row conversion (encoders / CatalystTypeConverters /
Dataset roundtrip), so the Connect + Arrow path is deliberately left out here and
is tracked by follow-up sub-tasks under the umbrella SPARK-56822:
- Arrow type mapping (`ArrowUtils.toArrowType` / `fromArrowType` - the
`unsupportedDataTypeError` you hit): SPARK-57159 "Add Arrow type mapping for
nanosecond-capable timestamp types".
- The encoder <-> vector dispatch in `ArrowSerializer` / `ArrowDeserializer`
(the `Unsupported Encoder/Vector` RuntimeException) plus the end-to-end Connect
flow: SPARK-57160 "Add Spark Connect protocol support for nanosecond-capable
timestamp types and literals" and SPARK-57161 "Convert nanosecond-capable
timestamp types and literals between proto and Catalyst in Spark Connect".
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56158:
URL: https://github.com/apache/spark/pull/56158#issuecomment-4563557832
@stevomitric Could you review this PR, please.
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3319906839
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -97,6 +99,10 @@ object RowEncoder extends DataTypeErrorsBase {
case TimestampType if SqlApiConf.get.datetimeJava8ApiEnabled => InstantEncoder(lenient)
case TimestampType => TimestampEncoder(lenient)
case TimestampNTZType => LocalDateTimeEncoder
+ // Nano timestamp types intentionally do not honor `lenient`: legacy `java.sql.Timestamp` /
+ // `java.sql.Date` external types are out of scope for nanosecond precision (SPARK-57033).
+ case t: TimestampNTZNanosType => LocalDateTimeNanosEncoder(t.precision)
Review Comment:
addressed
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "stevomitric (via GitHub)" <gi...@apache.org>.
stevomitric commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3318883129
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -97,6 +99,10 @@ object RowEncoder extends DataTypeErrorsBase {
case TimestampType if SqlApiConf.get.datetimeJava8ApiEnabled => InstantEncoder(lenient)
case TimestampType => TimestampEncoder(lenient)
case TimestampNTZType => LocalDateTimeEncoder
+ // Nano timestamp types intentionally do not honor `lenient`: legacy `java.sql.Timestamp` /
+ // `java.sql.Date` external types are out of scope for nanosecond precision (SPARK-57033).
+ case t: TimestampNTZNanosType => LocalDateTimeNanosEncoder(t.precision)
+ case t: TimestampLTZNanosType => InstantNanosEncoder(t.precision)
Review Comment:
We can inject types here also? Or are you waiting for a refactor PR? Myabe it would be a better idea to do it now so places like this one don't pile up in the future.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala:
##########
@@ -298,7 +300,7 @@ object CatalystTypeConverters {
}
new GenericInternalRow(ar)
case other => throw new SparkIllegalArgumentException(
- errorClass = "_LEGACY_ERROR_TEMP_3219",
+ errorClass = "INVALID_EXTERNAL_VALUE",
Review Comment:
Why rename the legacy error in this PR?
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -97,6 +99,10 @@ object RowEncoder extends DataTypeErrorsBase {
case TimestampType if SqlApiConf.get.datetimeJava8ApiEnabled => InstantEncoder(lenient)
case TimestampType => TimestampEncoder(lenient)
case TimestampNTZType => LocalDateTimeEncoder
+ // Nano timestamp types intentionally do not honor `lenient`: legacy `java.sql.Timestamp` /
+ // `java.sql.Date` external types are out of scope for nanosecond precision (SPARK-57033).
+ case t: TimestampNTZNanosType => LocalDateTimeNanosEncoder(t.precision)
Review Comment:
This looks like it bypasses the `spark.sql.timestampNanosTypes.enabled` gate.
`CatalystTypeConverters` still calls `TypeUtils.failUnsupportedDataType(...)`, so schema-driven converter paths reject nanos timestamp types when the feature is disabled. But `RowEncoder` now maps `TimestampNTZNanosType` / `TimestampLTZNanosType` to nanos encoders unconditionally, which means `Encoders.row(schema)` / `ExpressionEncoder(schema)` can succeed with the flag off.
Can we add the same gate here (or centralize the unsupported-type check) and add a negative test for the flag-off case?
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -789,6 +789,29 @@ public void testLocalDateTimeEncoder() {
Assertions.assertEquals(data, ds.collectAsList());
}
+ @Test
+ public void testTimestampNanosRowEncoder() {
+ spark.conf().set("spark.sql.timestampNanosTypes.enabled", "true");
Review Comment:
Do we need to explicitly set this? Its enabled in tests by default.
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3319834173
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -97,6 +99,10 @@ object RowEncoder extends DataTypeErrorsBase {
case TimestampType if SqlApiConf.get.datetimeJava8ApiEnabled => InstantEncoder(lenient)
case TimestampType => TimestampEncoder(lenient)
case TimestampNTZType => LocalDateTimeEncoder
+ // Nano timestamp types intentionally do not honor `lenient`: legacy `java.sql.Timestamp` /
+ // `java.sql.Date` external types are out of scope for nanosecond precision (SPARK-57033).
+ case t: TimestampNTZNanosType => LocalDateTimeNanosEncoder(t.precision)
+ case t: TimestampLTZNanosType => InstantNanosEncoder(t.precision)
Review Comment:
Let's do that by https://issues.apache.org/jira/browse/SPARK-57101
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3319701897
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala:
##########
@@ -298,7 +300,7 @@ object CatalystTypeConverters {
}
new GenericInternalRow(ar)
case other => throw new SparkIllegalArgumentException(
- errorClass = "_LEGACY_ERROR_TEMP_3219",
+ errorClass = "INVALID_EXTERNAL_VALUE",
Review Comment:
I do believe it is related to the changes.
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56158:
URL: https://github.com/apache/spark/pull/56158#issuecomment-4571411267
@cloud-fan @gengliangwang Could you review this PR, please.
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #56158: [SPARK-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps
URL: https://github.com/apache/spark/pull/56158
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56158:
URL: https://github.com/apache/spark/pull/56158#issuecomment-4587486761
Merging to master/4.x. Thank you, @stevomitric and @peter-toth 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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3330289473
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -97,6 +99,14 @@ object RowEncoder extends DataTypeErrorsBase {
case TimestampType if SqlApiConf.get.datetimeJava8ApiEnabled => InstantEncoder(lenient)
case TimestampType => TimestampEncoder(lenient)
case TimestampNTZType => LocalDateTimeEncoder
+ // Nano timestamp types intentionally do not honor `lenient`: legacy `java.sql.Timestamp` /
+ // `java.sql.Date` external types are out of scope for nanosecond precision (SPARK-57033).
+ case t: TimestampNTZNanosType =>
Review Comment:
The PR's `DatasetSuite` / `JavaDatasetSuite` tests confirm `createDataFrame(rows, schemaWithNanos).collect()` works under classic mode. The same call under Spark Connect throws before it ever reaches `collect()`:
- `sql/api/src/main/scala/org/apache/spark/sql/util/ArrowUtils.scala:51-77` (`toArrowTypeDefault`) has no `TimestampNTZNanosType` / `TimestampLTZNanosType` case — falls to `case _ => throw ExecutionErrors.unsupportedDataTypeError(dt)` (line 75-76). Server-side schema → Arrow type construction fails.
- `sql/connect/common/src/main/scala/org/apache/spark/sql/connect/client/arrow/ArrowSerializer.scala:524-525` and `.../ArrowDeserializer.scala:437-439` have no cases for the new `LocalDateTimeNanosEncoder` / `InstantNanosEncoder` — fall to `case _ => throw new RuntimeException(s"Unsupported Encoder($encoder)/Vector($v) combination.")`. Even if the Arrow schema mapping were added, the encoder ↔ vector dispatch would still fail.
Is the Connect / Arrow integration tracked in a follow-up ticket?
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3319701897
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala:
##########
@@ -298,7 +300,7 @@ object CatalystTypeConverters {
}
new GenericInternalRow(ar)
case other => throw new SparkIllegalArgumentException(
- errorClass = "_LEGACY_ERROR_TEMP_3219",
+ errorClass = "INVALID_EXTERNAL_VALUE",
Review Comment:
I do believe it is related to the changes. We postpone this issue every time while adding new types. I think it is worth to assign proper name now.
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #56158:
URL: https://github.com/apache/spark/pull/56158#discussion_r3319845524
##########
sql/core/src/test/java/test/org/apache/spark/sql/JavaDatasetSuite.java:
##########
@@ -789,6 +789,29 @@ public void testLocalDateTimeEncoder() {
Assertions.assertEquals(data, ds.collectAsList());
}
+ @Test
+ public void testTimestampNanosRowEncoder() {
+ spark.conf().set("spark.sql.timestampNanosTypes.enabled", "true");
Review Comment:
Removed the explicit spark.conf().set(...) / try-finally / spark.conf().unset(...) since spark.sql.timestampNanosTypes.enabled defaults to Utils.isTesting (true) in the test environment. The test body is unchanged, just unwrapped.
--
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-57033][SQL] Add java.time LocalDateTime/Instant conversion and Dataset roundtrip for nanosecond timestamps [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #56158:
URL: https://github.com/apache/spark/pull/56158#issuecomment-4581813096
@peter-toth @beliefer Could you review this PR, please.
--
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