You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ayushtkn (via GitHub)" <gi...@apache.org> on 2026/01/06 13:35:28 UTC
[PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
ayushtkn opened a new pull request, #14974:
URL: https://github.com/apache/iceberg/pull/14974
Fixes: https://github.com/apache/iceberg/issues/14973
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3715239568
I know that Flink is not supporting nano yet. See: #14245
I surprised though that Spark does not support nano. What is the exact exception? Maybe only the Spark wrapper has issues with wrapping nano timestamps.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3718379785
> My first idea to create a TestInternalRecordWrapper which extends ` and the generateAndValidate would only check if the wrapping was successful? The test would be overwritten by the Spark and Flink tests like it is done for TestInternalRowWrapper.testTimestampWithoutZone.
Thanx @pvary I have added a new test `TestInternalRecordWrapper` which extends `RecordWrapperTestBase` and validates the type is correct. I checked without my fix the tests fail as well
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#discussion_r2665329033
##########
data/src/test/java/org/apache/iceberg/data/TestGenericRecord.java:
##########
@@ -83,4 +86,35 @@ public void testDeepCopyValues() {
assertThat(copy.getField(field.name())).isNotSameAs(original.getField(field.name()));
}
}
+
+ @Test
+ public void testInternalRecordWrapperWithTimestampNano() {
Review Comment:
Maybe we should add a test to `RecordWrapperTestBase` instead?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#discussion_r2668059868
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/TestRowDataWrapper.java:
##########
@@ -59,6 +60,18 @@ public void testTime() {
});
}
+ @Disabled
+ @Override
+ public void testTimestampNanoWithoutZone() {
+ // Flink does not support timestamp without zone.
Review Comment:
```suggestion
// Flink does not support nanosecond timestamp without zone.
```
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/TestRowDataWrapper.java:
##########
@@ -59,6 +60,18 @@ public void testTime() {
});
}
+ @Disabled
+ @Override
+ public void testTimestampNanoWithoutZone() {
+ // Flink does not support timestamp without zone.
+ }
+
+ @Disabled
+ @Override
+ public void testTimestampNanoWithZone() {
+ // Flink does not support timestamp without zone.
Review Comment:
```suggestion
// Flink does not support nanosecond timestamp with zone.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3717897774
Thanks @ayushtkn for the investigation.
My first idea to create a `TestInternalRecordWrapper` which extends `RecordWrapperTestBase` and the `generateAndValidate` would only check if the wrapping was successful? The test would be overwritten by the Spark and Flink tests like it is done for `TestInternalRowWrapper.testTimestampWithoutZone`.
Alternatively we can create a separate `TestInternalRecordWrapper` which tests specific conversions like `DATE`, `TIME` etc, and maybe tests anything else as a noop?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#discussion_r2668062744
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/TestRowDataWrapper.java:
##########
@@ -59,6 +60,18 @@ public void testTime() {
});
}
+ @Disabled
+ @Override
+ public void testTimestampNanoWithoutZone() {
+ // Flink does not support timestamp without zone.
Review Comment:
Same for the other Flink versions
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3715476753
So it is not supported in Spark, not supported in Flink.
Where is it supported?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #14974:
URL: https://github.com/apache/iceberg/pull/14974
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#discussion_r2668060981
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/TestRowDataWrapper.java:
##########
@@ -59,6 +60,18 @@ public void testTime() {
});
}
+ @Disabled
+ @Override
+ public void testTimestampNanoWithoutZone() {
+ // Flink does not support timestamp without zone.
+ }
+
+ @Disabled
+ @Override
+ public void testTimestampNanoWithZone() {
+ // Flink does not support timestamp without zone.
Review Comment:
```suggestion
// Flink does not support nano timestamp with zone.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#discussion_r2668059868
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/TestRowDataWrapper.java:
##########
@@ -59,6 +60,18 @@ public void testTime() {
});
}
+ @Disabled
+ @Override
+ public void testTimestampNanoWithoutZone() {
+ // Flink does not support timestamp without zone.
Review Comment:
```suggestion
// Flink does not support nano timestamp without zone.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3715535666
I am implementing in Hive, It is complete, but couple of my query fails bcoz of this. It relies on this code & Since TimeStampNano isn't there it doesn't converts it into Long
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#discussion_r2665350423
##########
data/src/test/java/org/apache/iceberg/data/TestGenericRecord.java:
##########
@@ -83,4 +86,35 @@ public void testDeepCopyValues() {
assertThat(copy.getField(field.name())).isNotSameAs(original.getField(field.name()));
}
}
+
+ @Test
+ public void testInternalRecordWrapperWithTimestampNano() {
Review Comment:
I did try writing one, but the child class Spark says the type isn't supported and the flink one
```
diff --git a/data/src/test/java/org/apache/iceberg/RecordWrapperTestBase.java b/data/src/test/java/org/apache/iceberg/RecordWrapperTestBase.java
index f51d1ebed..ef6ec9cb8 100644
--- a/data/src/test/java/org/apache/iceberg/RecordWrapperTestBase.java
+++ b/data/src/test/java/org/apache/iceberg/RecordWrapperTestBase.java
@@ -52,6 +52,11 @@ public abstract class RecordWrapperTestBase {
required(101, "ts0", Types.TimestampType.withoutZone()),
required(102, "ts1", Types.TimestampType.withoutZone()));
+ private static final Types.StructType TIMESTAMP_NS_WITHOUT_ZONE =
+ Types.StructType.of(
+ required(101, "ts0", Types.TimestampNanoType.withoutZone()),
+ required(102, "ts1", Types.TimestampNanoType.withoutZone()));
+
protected static final Types.StructType TIME =
Types.StructType.of(
required(100, "time0", Types.TimeType.get()),
@@ -67,6 +72,11 @@ public abstract class RecordWrapperTestBase {
generateAndValidate(new Schema(TIMESTAMP_WITHOUT_ZONE.fields()));
}
+ @Test
+ public void testTimestampNaNoWithoutZone() {
+ generateAndValidate(new Schema(TIMESTAMP_NS_WITHOUT_ZONE.fields()));
+ }
+
@Test
public void testTime() {
generateAndValidate(new Schema(TIME.fields()));
```
And the Flink return something different, I started debugging bit on flink side:
```
TestRowDataWrapper > testTimestampNaNoWithoutZone() FAILED
java.lang.AssertionError: StructLike mismatch detected
Field [0] name=ts0 type=timestamp_ns
expected=org.apache.iceberg.flink.RowDataWrapper@16423501 (org.apache.iceberg.flink.RowDataWrapper)
actual =org.apache.iceberg.data.InternalRecordWrapper@4efcf8a (org.apache.iceberg.data.InternalRecordWrapper)
Field [1] name=ts1 type=timestamp_ns
expected=org.apache.iceberg.flink.RowDataWrapper@16423501 (org.apache.iceberg.flink.RowDataWrapper)
actual =org.apache.iceberg.data.InternalRecordWrapper@4efcf8a (org.apache.iceberg.data.InternalRecordWrapper)
at org.apache.iceberg.flink.TestRowDataWrapper.generateAndValidate(TestRowDataWrapper.java:119)
```
I added bit more assertions, it returns `RowDataWrapper`
I was trying on Hive & it was failing with same exception while creating a `GenericRecord`, So I added it here, It didn't had any Hive child class,
Let me know if I missed something or did something wrong
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3715298641
This is from Spark
```
TestInternalRowWrapper > testTimestampNaNoWithoutZone() FAILED
java.lang.UnsupportedOperationException: Cannot convert unsupported type to Spark: timestamp_ns
at org.apache.iceberg.spark.TypeToSparkType.primitive(TypeToSparkType.java:154)
at org.apache.iceberg.spark.TypeToSparkType.primitive(TypeToSparkType.java:50)
at org.apache.iceberg.types.TypeUtil.visit(TypeUtil.java:732)
at org.apache.iceberg.types.TypeUtil.visit(TypeUtil.java:683)
at org.apache.iceberg.types.TypeUtil.visit(TypeUtil.java:671)
at org.apache.iceberg.spark.SparkSchemaUtil.convert(SparkSchemaUtil.java:96)
at org.apache.iceberg.spark.source.TestInternalRowWrapper.generateAndValidate(TestInternalRowWrapper.java:58)
```
On the website also it is mentioned it is not supported in Spark
<img width="621" height="158" alt="image" src="https://github.com/user-attachments/assets/5fe2f382-9eca-4ba7-b0c9-0db46f2a3a0d" />
https://iceberg.apache.org/docs/latest/spark-getting-started/#iceberg-type-to-spark-type
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3718461081
@ayushtkn: Looks good to me. Please fix the comments in the Flink tests, and then this is good to go.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3719014683
> Please fix the comments in the Flink tests, and then this is good to go.
Thanx @pvary for the review. I have fixed the comments in the tests
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Handle TIMESTAMP_NANO in InternalRecordWrapper [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14974:
URL: https://github.com/apache/iceberg/pull/14974#issuecomment-3719168021
Merged to main.
Thanks @ayushtkn for the 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org