From efe1d1477ad7e5daaf3d16fc021aa018e78b0ef7 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Fri, 13 Dec 2024 15:08:45 +0900 Subject: [PATCH 1/3] Core: Fix numeric overflow of timestamp nano literal --- .../apache/iceberg/expressions/Literals.java | 3 +-- .../TestTimestampLiteralConversions.java | 23 +++++++++++++++++++ .../apache/iceberg/types/TestConversions.java | 4 ++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/expressions/Literals.java b/api/src/main/java/org/apache/iceberg/expressions/Literals.java index ee47035b1e72..88fcab633812 100644 --- a/api/src/main/java/org/apache/iceberg/expressions/Literals.java +++ b/api/src/main/java/org/apache/iceberg/expressions/Literals.java @@ -300,8 +300,7 @@ public Literal to(Type type) { case TIMESTAMP: return (Literal) new TimestampLiteral(value()); case TIMESTAMP_NANO: - // assume micros and convert to nanos to match the behavior in the timestamp case above - return new TimestampLiteral(value()).to(type); + return (Literal) new TimestampNanoLiteral(value()); case DATE: if ((long) Integer.MAX_VALUE < value()) { return aboveMax(); diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java b/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java index 379ad4db5e97..c12375e1ce28 100644 --- a/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java +++ b/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java @@ -21,6 +21,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.time.LocalDate; import java.time.format.DateTimeParseException; import org.apache.iceberg.types.Types; import org.apache.iceberg.util.DateTimeUtil; @@ -107,6 +108,28 @@ public void testTimestampMicrosToDateConversion() { assertThat(dateOrdinal).isEqualTo(-1); } + @Test + public void testTimestampNanoWithLongLiteral() { + // verify round-trip between timestamp_ns and long + Literal timestampNano = + Literal.of("2017-11-16T14:31:08.000000001").to(Types.TimestampNanoType.withoutZone()); + assertThat(timestampNano.value()).isEqualTo(1510842668000000001L); + + Literal longLiteral = + Literal.of(1510842668000000001L).to(Types.TimestampNanoType.withoutZone()); + assertThat(longLiteral).isEqualTo(timestampNano); + + // cast long literal to temporal types + assertThat(longLiteral.to(Types.DateType.get()).value()) + .isEqualTo((int) LocalDate.of(2017, 11, 16).toEpochDay()); + + assertThat(longLiteral.to(Types.TimestampType.withoutZone()).value()) + .isEqualTo(1510842668000000L); + + assertThat(longLiteral.to(Types.TimestampNanoType.withoutZone()).value()) + .isEqualTo(1510842668000000001L); + } + @Test public void testTimestampNanoToTimestampConversion() { Literal timestamp = diff --git a/api/src/test/java/org/apache/iceberg/types/TestConversions.java b/api/src/test/java/org/apache/iceberg/types/TestConversions.java index e207cfd8d59a..68636490104e 100644 --- a/api/src/test/java/org/apache/iceberg/types/TestConversions.java +++ b/api/src/test/java/org/apache/iceberg/types/TestConversions.java @@ -111,9 +111,9 @@ public void testByteBufferConversions() { assertConversion( 400000000L, TimestampNanoType.withZone(), new byte[] {0, -124, -41, 23, 0, 0, 0, 0}); assertThat(Literal.of(400000L).to(TimestampNanoType.withoutZone()).toByteBuffer().array()) - .isEqualTo(new byte[] {0, -124, -41, 23, 0, 0, 0, 0}); + .isEqualTo(new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); assertThat(Literal.of(400000L).to(TimestampNanoType.withZone()).toByteBuffer().array()) - .isEqualTo(new byte[] {0, -124, -41, 23, 0, 0, 0, 0}); + .isEqualTo(new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); // strings are stored as UTF-8 bytes (without length) // 'A' -> 65, 'B' -> 66, 'C' -> 67 From 2a9c8b7fa11145fcef63b9377733b4966f6a7457 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Tue, 17 Dec 2024 08:04:23 +0900 Subject: [PATCH 2/3] fixup! Core: Fix numeric overflow of timestamp nano literal --- .../TestTimestampLiteralConversions.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java b/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java index c12375e1ce28..79e75b59a55c 100644 --- a/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java +++ b/api/src/test/java/org/apache/iceberg/expressions/TestTimestampLiteralConversions.java @@ -126,8 +126,13 @@ public void testTimestampNanoWithLongLiteral() { assertThat(longLiteral.to(Types.TimestampType.withoutZone()).value()) .isEqualTo(1510842668000000L); + assertThat(longLiteral.to(Types.TimestampType.withZone()).value()).isEqualTo(1510842668000000L); + assertThat(longLiteral.to(Types.TimestampNanoType.withoutZone()).value()) .isEqualTo(1510842668000000001L); + + assertThat(longLiteral.to(Types.TimestampNanoType.withZone()).value()) + .isEqualTo(1510842668000000001L); } @Test @@ -181,6 +186,33 @@ public void testTimestampNanosToDateConversion() { assertThat(dateOrdinal).isEqualTo(-1); } + @Test + public void testTimestampNanoWithZoneWithLongLiteral() { + // verify round-trip between timestamptz_ns and long + Literal timestampNanoWithZone = + Literal.of("2017-11-16T14:31:08.000000001+01:00").to(Types.TimestampNanoType.withZone()); + assertThat(timestampNanoWithZone.value()).isEqualTo(1510839068000000001L); + + Literal longLiteral = + Literal.of(1510839068000000001L).to(Types.TimestampNanoType.withZone()); + assertThat(longLiteral).isEqualTo(timestampNanoWithZone); + + // cast long literal to temporal types + assertThat(longLiteral.to(Types.DateType.get()).value()) + .isEqualTo((int) LocalDate.of(2017, 11, 16).toEpochDay()); + + assertThat(longLiteral.to(Types.TimestampType.withoutZone()).value()) + .isEqualTo(1510839068000000L); + + assertThat(longLiteral.to(Types.TimestampType.withZone()).value()).isEqualTo(1510839068000000L); + + assertThat(longLiteral.to(Types.TimestampNanoType.withoutZone()).value()) + .isEqualTo(1510839068000000001L); + + assertThat(longLiteral.to(Types.TimestampNanoType.withoutZone()).value()) + .isEqualTo(1510839068000000001L); + } + @Test public void testTimestampNanosWithZoneConversion() { Literal isoTimestampNanosWithZoneOffset = From 6828e0cea06242721002fbd33c13d7579033521a Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Tue, 17 Dec 2024 09:45:40 +0900 Subject: [PATCH 3/3] fixup! Core: Fix numeric overflow of timestamp nano literal --- .../test/java/org/apache/iceberg/types/TestConversions.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/src/test/java/org/apache/iceberg/types/TestConversions.java b/api/src/test/java/org/apache/iceberg/types/TestConversions.java index 68636490104e..00dc2f5df260 100644 --- a/api/src/test/java/org/apache/iceberg/types/TestConversions.java +++ b/api/src/test/java/org/apache/iceberg/types/TestConversions.java @@ -104,12 +104,10 @@ public void testByteBufferConversions() { .isEqualTo(new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); assertThat(Literal.of(400000L).to(TimestampType.withZone()).toByteBuffer().array()) .isEqualTo(new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); - // values passed to assertConversion and Literal.of differ because Literal.of(...) assumes - // the value is in micros, which gets converted when to(TimestampNanoType) is called assertConversion( - 400000000L, TimestampNanoType.withoutZone(), new byte[] {0, -124, -41, 23, 0, 0, 0, 0}); + 400000L, TimestampNanoType.withoutZone(), new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); assertConversion( - 400000000L, TimestampNanoType.withZone(), new byte[] {0, -124, -41, 23, 0, 0, 0, 0}); + 400000L, TimestampNanoType.withZone(), new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); assertThat(Literal.of(400000L).to(TimestampNanoType.withoutZone()).toByteBuffer().array()) .isEqualTo(new byte[] {-128, 26, 6, 0, 0, 0, 0, 0}); assertThat(Literal.of(400000L).to(TimestampNanoType.withZone()).toByteBuffer().array())