Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-31 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1698841407 ## api/src/main/java/org/apache/iceberg/expressions/Literals.java: ## @@ -299,6 +300,8 @@ public Literal to(Type type) { return (Literal) new

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-31 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2260512986 @rdblue I will address all your comments, but all but ONE of them are purely internal implementation details. Can we agree that it is safe to address those after merging this? The

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-31 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1698492880 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,159 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697473790 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,159 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697470881 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,159 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697462920 ## api/src/main/java/org/apache/iceberg/transforms/Bucket.java: ## @@ -214,6 +217,20 @@ protected int hash(Long value) { } } + // In order to bucket

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697462451 ## api/src/main/java/org/apache/iceberg/expressions/Literals.java: ## @@ -515,6 +548,18 @@ public Literal to(Type type) { return (Literal) new

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697461646 ## api/src/main/java/org/apache/iceberg/expressions/Literals.java: ## @@ -453,6 +458,34 @@ protected Type.TypeID typeId() { } } + static class

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697460829 ## api/src/main/java/org/apache/iceberg/expressions/Literals.java: ## @@ -299,6 +300,8 @@ public Literal to(Type type) { return (Literal) new

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697454916 ## api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java: ## @@ -515,6 +523,8 @@ private static String sanitize(Type type, Object value, long now, int

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-30 Thread via GitHub
rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1697454334 ## api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java: ## @@ -515,6 +523,8 @@ private static String sanitize(Type type, Object value, long now, int

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-29 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2255138973 I don't have anything else and yes #10775 needs to be handled before 1.7 (would be good to start work on it rather sooner than later) -- This is an automated message from the Apache Git

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-28 Thread via GitHub
amogh-jahagirdar commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2254623684 Thanks @jacobmarble @epgif it looks good from my analysis. I'll wait before merging tomorrow in case @nastra has anything else. We can merge this into main, but I'd say #10775

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-28 Thread via GitHub
amogh-jahagirdar commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1694311600 ## api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java: ## @@ -515,6 +523,8 @@ private static String sanitize(Type type, Object value, long

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-28 Thread via GitHub
amogh-jahagirdar commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1694308507 ## api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java: ## @@ -515,6 +523,8 @@ private static String sanitize(Type type, Object value, long

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-28 Thread via GitHub
amogh-jahagirdar commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1694308507 ## api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java: ## @@ -515,6 +523,8 @@ private static String sanitize(Type type, Object value, long

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-26 Thread via GitHub
amogh-jahagirdar commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2253718238 Yes, I'll take another pass tomorrow! Thanks for your patience @jacobmarble -- This is an automated message from the Apache Git Service. To respond to the message, please log

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-25 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2250721645 @amogh-jahagirdar would you like to take another look? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-24 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2248525449 @nastra what should be our expectation from here? Now that 1.6.0 is released, is this PR merged into main, or does someone else need to review it first? -- This is an automated

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-24 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2248517382 > > > Given that this type is for v3, wouldn't a v2 writer just blindly write this new type if it's being used anywhere and thus break forward compability? > > > > > >

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-11 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2223123048 > overall this LGTM once comments in `TestBucketing` have been addressed I've addressed these. Thanks! -- This is an automated message from the Apache Git Service. To

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-11 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1674135937 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -165,6 +159,68 @@ public void testLong() { .isEqualTo(hashBytes(buffer.array()));

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-11 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1674135249 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -165,6 +159,68 @@ public void testLong() { .isEqualTo(hashBytes(buffer.array()));

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-11 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1674134865 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,6 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-11 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-134088 I've marked this PR to go into 1.7.0. @jacobmarble could you please open a follow-up issue to tackle what I mentioned in

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-10 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1672474232 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -165,6 +159,68 @@ public void testLong() { .isEqualTo(hashBytes(buffer.array()));

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-10 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1672470780 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -165,6 +159,68 @@ public void testLong() { .isEqualTo(hashBytes(buffer.array()));

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-10 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1672464813 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -165,6 +159,68 @@ public void testLong() { .isEqualTo(hashBytes(buffer.array()));

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-10 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1672454237 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,6 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1670738516 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -132,6 +132,51 @@ public void testStringToTimestampLiteral() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1670683308 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,59 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1670683949 ## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ## @@ -75,6 +84,14 @@ public static long microsToMillis(long micros) { return Math.floorDiv(micros,

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1670684812 ## api/src/test/java/org/apache/iceberg/util/TestDateTimeUtil.java: ## @@ -35,4 +36,36 @@ public void formatTimestampMillis() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1670684480 ## api/src/test/java/org/apache/iceberg/util/TestDateTimeUtil.java: ## @@ -35,4 +36,36 @@ public void formatTimestampMillis() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1670683308 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,59 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2216824071 > > Given that this type is for v3, wouldn't a v2 writer just blindly write this new type if it's being used anywhere and thus break forward compability? > > @nastra I don't think I

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-09 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669903058 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,59 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669424764 ## api/src/test/java/org/apache/iceberg/transforms/TestYears.java: ## @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669424619 ## api/src/test/java/org/apache/iceberg/transforms/TestYears.java: ## @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669412029 ## api/src/test/java/org/apache/iceberg/types/TestTypes.java: ## @@ -31,6 +31,8 @@ public void fromPrimitiveString() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669410065 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669416443 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -181,17 +226,30 @@ public void testNegativeStringToTimestampLiteral()

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669413492 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -181,17 +226,30 @@ public void testNegativeStringToTimestampLiteral()

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669412029 ## api/src/test/java/org/apache/iceberg/types/TestTypes.java: ## @@ -31,6 +31,8 @@ public void fromPrimitiveString() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669410065 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2215448506 > Given that this type is for v3, wouldn't a v2 writer just blindly write this new type if it's being used anywhere and thus break forward compability? @nastra I don't think I

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669176686 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,59 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1669075324 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -132,6 +132,51 @@ public void testStringToTimestampLiteral() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
amogh-jahagirdar commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668793711 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -132,6 +132,51 @@ public void testStringToTimestampLiteral() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668575389 ## api/src/test/java/org/apache/iceberg/util/TestDateTimeUtil.java: ## @@ -35,4 +36,36 @@ public void formatTimestampMillis() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668565732 ## api/src/test/java/org/apache/iceberg/util/TestDateTimeUtil.java: ## @@ -35,4 +36,36 @@ public void formatTimestampMillis() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668555332 ## api/src/test/java/org/apache/iceberg/util/TestDateTimeUtil.java: ## @@ -35,4 +36,36 @@ public void formatTimestampMillis() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668548266 ## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ## @@ -75,6 +84,14 @@ public static long microsToMillis(long micros) { return Math.floorDiv(micros,

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668546405 ## api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java: ## @@ -112,12 +112,59 @@ public void testSpecValues() { .as("Spec example:

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668515434 ## api/src/test/java/org/apache/iceberg/transforms/TestDays.java: ## @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668513890 ## api/src/test/java/org/apache/iceberg/transforms/TestYears.java: ## @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668507190 ## api/src/test/java/org/apache/iceberg/types/TestTypes.java: ## @@ -31,6 +31,8 @@ public void fromPrimitiveString() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668502299 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668495939 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -181,17 +226,30 @@ public void testNegativeStringToTimestampLiteral() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668495396 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -181,17 +226,30 @@ public void testNegativeStringToTimestampLiteral() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-08 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1668493077 ## api/src/test/java/org/apache/iceberg/expressions/TestStringLiteralConversions.java: ## @@ -132,6 +132,51 @@ public void testStringToTimestampLiteral() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2208073422 Thanks @amogh-jahagirdar! I believe I've addressed all the comments. Please note that @jacobmarble and I are both in the US and will be out of the office Thursday and Friday.

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1665079938 ## api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java: ## @@ -594,6 +605,12 @@ private static String sanitizeString(CharSequence value, long now, int

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1665079821 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,160 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1665080227 ## api/src/test/java/org/apache/iceberg/TestPartitionPaths.java: ## @@ -54,6 +54,25 @@ public void testPartitionPath() {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1665080053 ## api/src/main/java/org/apache/iceberg/types/Types.java: ## @@ -259,6 +261,60 @@ public int hashCode() { } } + public static class TimestampNanoType extends

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
amogh-jahagirdar commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1664436827 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,160 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-07-03 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2206491863 Hey @jacobmarble thanks for being patient here. There's no particular concern but I've been busy with other things and it was difficult to find a big block of time to re-review this PR.

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-18 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2176351708 > @jacobmarble I'll try to do another round this week once merge conflicts have been resolved. Also I think it would be good to get a review from @rdblue on this one Conflicts

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-18 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2175917761 @jacobmarble I'll try to do another round this week once merge conflicts have been resolved. Also I think it would be good to get a review from @rdblue on this one -- This is an

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-17 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2173664606 @nastra is there anything else to do on this pull request before merging? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2153003145 Thank you for the quick review @nastra! I believe everything is addressed now. Please have another look when you can. Thanks! -- This is an automated message from the Apache Git

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629896432 ## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ## @@ -185,7 +194,28 @@ private static int convertMicros(long micros, ChronoUnit granularity) { }

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629895942 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,162 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629893300 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,162 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629892323 ## api/src/main/java/org/apache/iceberg/transforms/SortOrderVisitor.java: ## @@ -85,21 +85,18 @@ static List visit(SortOrder sortOrder, SortOrderVisitor visitor) {

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629892623 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629891562 ## api/src/main/java/org/apache/iceberg/transforms/Transforms.java: ## @@ -188,9 +198,15 @@ public static Transform day(Type type) { @Deprecated

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629892839 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
epgif commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629891910 ## api/src/main/java/org/apache/iceberg/transforms/Days.java: ## @@ -55,7 +57,16 @@ public boolean satisfiesOrderOf(Transform other) { } if (other instanceof

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
findepi commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629836495 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629728440 ## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ## @@ -185,7 +194,28 @@ private static int convertMicros(long micros, ChronoUnit granularity) { }

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629725179 ## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ## @@ -185,7 +194,28 @@ private static int convertMicros(long micros, ChronoUnit granularity) { }

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629722207 ## api/src/test/java/org/apache/iceberg/transforms/TestDays.java: ## @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629719484 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,162 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629246525 ## api/src/test/java/org/apache/iceberg/transforms/TestYears.java: ## @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629248863 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,162 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629247791 ## api/src/main/java/org/apache/iceberg/transforms/Timestamps.java: ## @@ -31,54 +33,162 @@ import org.apache.iceberg.util.DateTimeUtil; import

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629245294 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629244450 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-06 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1629238344 ## api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java: ## @@ -241,4 +243,54 @@ public void testTimestampsReturnType() { Type hourResultType =

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-05 Thread via GitHub
nastra commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1627706644 ## api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java: ## @@ -75,6 +77,14 @@ public static long microsToMillis(long micros) { return Math.floorDiv(micros,

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-06-04 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2147874958 Friendly ping @nastra @rdblue :) -- 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

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-05-28 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2135579590 @nastra or @rdblue - friendly reminder to review this -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-05-21 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2123288831 This should be ready to go. Thanks! -- 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

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-05-20 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2121078774 @nastra is this a good week for you to 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

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-05-06 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2097095003 @jacobmarble sorry for the delay here. I'm traveling this week and should be able to get to this PR after the Iceberg summit -- This is an automated message from the Apache Git Service.

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-05-06 Thread via GitHub
jacobmarble commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2096395819 > @epgif can you please address the test failures? @nastra do you intend to review this pull request further? -- This is an automated message from the Apache Git Service. To

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-04-25 Thread via GitHub
epgif commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2077790169 > @epgif can you please address the test failures? Done. Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

Re: [PR] API: implement types timestamp_ns and timestamptz_ns [iceberg]

2024-04-25 Thread via GitHub
nastra commented on PR #9008: URL: https://github.com/apache/iceberg/pull/9008#issuecomment-2077042155 @epgif can you please address the test failures? -- 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

  1   2   >