aihuaxu commented on code in PR #12682:
URL: https://github.com/apache/iceberg/pull/12682#discussion_r2052976424
##########
api/src/main/java/org/apache/iceberg/variants/PhysicalType.java:
##########
@@ -41,6 +41,10 @@ public enum PhysicalType {
FLOAT(LogicalType.FLOAT, Float.class),
BINARY(LogicalType.BINARY, ByteBuffer.class),
STRING(LogicalType.STRING, String.class),
+ TIMENTZ(LogicalType.TIMENTZ, Long.class),
+ TIMESTAMPTZNS(LogicalType.TIMESTAMPTZ, Long.class),
+ TIMESTAMPNTZNS(LogicalType.TIMESTAMPNTZ, Long.class),
Review Comment:
That sounds better. Thanks.
##########
api/src/test/java/org/apache/iceberg/variants/TestSerializedPrimitives.java:
##########
@@ -451,11 +452,143 @@ public void testShortString() {
assertThat(value.get()).isEqualTo("iceberg");
}
+ @Test
+ public void testTimentzNano() {
Review Comment:
Negative is not allowed with "Invalid value for NanoOfDay (valid values 0 -
86399999999999)". I can add one for this.
```
Invalid value for NanoOfDay (valid values 0 - 86399999999999): -1000
java.time.DateTimeException: Invalid value for NanoOfDay (valid values 0 -
86399999999999): -1000
at
java.base/java.time.temporal.ValueRange.checkValidValue(ValueRange.java:319)
at
java.base/java.time.temporal.ChronoField.checkValidValue(ChronoField.java:718)
at java.base/java.time.LocalTime.ofNanoOfDay(LocalTime.java:408)
at
org.apache.iceberg.util.DateTimeUtil.timeFromMicros(DateTimeUtil.java:62)
at
org.apache.iceberg.variants.TestSerializedPrimitives.testTimentzNanoNegative(TestSerializedPrimitives.java:493)
```
##########
api/src/main/java/org/apache/iceberg/variants/VariantObject.java:
##########
@@ -44,9 +46,13 @@ default VariantObject asObject() {
static String asString(VariantObject object) {
StringBuilder builder = new StringBuilder();
+ // Make the string deterministic
Review Comment:
This is from the test
https://github.com/apache/iceberg/blob/main/core/src/test/java/org/apache/iceberg/avro/TestAvroEncoderUtil.java#L62.
In variant spec, we are sorting field ids. So we are creating a record in
original order while we are reading back as a different order with field ids
sorted.
`The field ids and field offsets must be in lexicographical order of the
corresponding field names in the metadata dictionary. However, the actual value
entries do not need to be in any particular order.
`
##########
core/src/main/java/org/apache/iceberg/variants/Variants.java:
##########
@@ -200,4 +201,36 @@ public static VariantPrimitive<ByteBuffer> of(ByteBuffer
value) {
public static VariantPrimitive<String> of(String value) {
return new PrimitiveWrapper<>(PhysicalType.STRING, value);
}
+
+ public static VariantPrimitive<Long> ofTimentz(long value) {
+ return new PrimitiveWrapper<>(PhysicalType.TIMENTZ, value);
+ }
+
+ public static VariantPrimitive<Long> ofIsoTimentz(String value) {
+ return ofTimentz(DateTimeUtil.isoTimeToMicros(value));
+ }
+
+ public static VariantPrimitive<Long> ofTimestamptzNano(long value) {
+ return new PrimitiveWrapper<>(PhysicalType.TIMESTAMPTZNS, value);
+ }
+
+ public static VariantPrimitive<Long> ofIsoTimestamptzNano(String value) {
+ return ofTimestamptzNano(DateTimeUtil.isoTimestamptzToNanos(value));
+ }
+
+ public static VariantPrimitive<Long> ofTimestampntzNano(long value) {
+ return new PrimitiveWrapper<>(PhysicalType.TIMESTAMPNTZNS, value);
+ }
+
+ public static VariantPrimitive<Long> ofIsoTimestampntzNano(String value) {
+ return ofTimestampntzNano(DateTimeUtil.isoTimestampToNanos(value));
+ }
+
+ public static VariantPrimitive<UUID> ofUuid(UUID uuid) {
Review Comment:
Thanks. I think the `ofUuid(String uuid)` should too, right?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
