You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/12/22 07:25:39 UTC

[GitHub] [iceberg] zhangbutao opened a new pull request, #6482: API: Fix inconsistent TimeTransform Type

zhangbutao opened a new pull request, #6482:
URL: https://github.com/apache/iceberg/pull/6482

   After https://github.com/apache/iceberg/pull/5601 and https://github.com/apache/iceberg/pull/6220, there is inconsistent `TimeTransform Type` in some codes. This causes an exception when remove and then add a same time partition(year, month, day, hour).
   
   I added a UT in this pr, and you can use the UT to reproduce this issue. This issue existed in iceberg1.1.0 and it blocked me upgrade iceberg from 1.0.0 to 1.1.0 in Hive repo. https://github.com/apache/hive/pull/3851 and failed tests http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-3851/1/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


[GitHub] [iceberg] zhangbutao commented on pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by GitBox <gi...@apache.org>.
zhangbutao commented on PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#issuecomment-1363497987

   I am new to iceberg. Can anyone give approval to run the CI? 


-- 
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


[GitHub] [iceberg] zhangbutao commented on pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#issuecomment-1407247517

   Sorry for long time to come back. Will address comments as soon as :) @hililiwei 


-- 
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


[GitHub] [iceberg] zhangbutao commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1089690341


##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());
+
+    //  remove and add a field with TimeTransform(Years, Months, Days, Hours)
+    table.updateSpec().removeField("year_field_year").addField(year("year_field")).commit();
+
+    V1Assert.assertEquals(
+            "Should remove and then add a year field",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .bucket("data", 16)
+                    .year("year_field")
+                    .build(),
+            table.spec());
+    V2Assert.assertEquals(

Review Comment:
   1. test1: adding a year field to the spec(`table.updateSpec().addField(year("year_field")).commit()`;)
      I want to verify if the actual table transform type is same as `PartitionSpec.builderFor` and the transform type class should be `org.apache.iceberg.transforms.Years`. If not same, the test2 will also faill.
   
   2. test2: `table.updateSpec().removeField("year_field_year").addField(year("year_field")).commit()`;
      Because i have encountered exception when **removed and then add a same field**, i want to check if this could work after the fix. To be exact, test2  is a e2e test. 



-- 
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


[GitHub] [iceberg] Fokko commented on pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#issuecomment-1414044966

   Thanks for letting me know and creating the PR in the first place, much appreciated 👍🏻 


-- 
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


[GitHub] [iceberg] nastra commented on pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by GitBox <gi...@apache.org>.
nastra commented on PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#issuecomment-1375510520

   Note that this PR is essentially reverting https://github.com/apache/iceberg/pull/6220, which I don't think we can do atm. /cc @Fokko 


-- 
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


[GitHub] [iceberg] zhangbutao commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1089692889


##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());

Review Comment:
   Added, thx



-- 
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


[GitHub] [iceberg] zhangbutao commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1089692854


##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());
+
+    //  remove and add a field with TimeTransform(Years, Months, Days, Hours)
+    table.updateSpec().removeField("year_field_year").addField(year("year_field")).commit();

Review Comment:
   I don't think we need this case. Other tests have include the case, eg. `testAddAfterLastFieldRemoved`



-- 
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


[GitHub] [iceberg] Fokko closed pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko closed pull request #6482: API: Fix inconsistent TimeTransform Type
URL: https://github.com/apache/iceberg/pull/6482


-- 
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


[GitHub] [iceberg] zhangbutao commented on pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#issuecomment-1407353080

   > Note that this PR is essentially reverting #6220, which I don't think we can do atm. /cc @Fokko
   
   @rdblue @Fokko @nastra Could you please take a look? I am not sure if this fix will revert https://github.com/apache/iceberg/pull/6220. In addition, another PR https://github.com/apache/iceberg/pull/6653 is aim for same issue. 


-- 
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


[GitHub] [iceberg] Fokko commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1094693759


##########
api/src/main/java/org/apache/iceberg/PartitionSpec.java:
##########
@@ -440,7 +440,7 @@ public Builder year(String sourceName, String targetName) {
               sourceColumn.fieldId(),
               nextFieldId(),
               targetName,
-              Transforms.year(sourceColumn.type()));

Review Comment:
   I'm hesitant to remove those because then there is a situation where we don't know the `sourceColumn`.



-- 
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


[GitHub] [iceberg] hililiwei commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by GitBox <gi...@apache.org>.
hililiwei commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1056334660


##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());

Review Comment:
   This seems superfluous; the previous one is enough? If needed, we can add 
   `Assert.assertEquals(1001, table.spec().lastAssignedFieldId());`



##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());
+
+    //  remove and add a field with TimeTransform(Years, Months, Days, Hours)
+    table.updateSpec().removeField("year_field_year").addField(year("year_field")).commit();
+
+    V1Assert.assertEquals(
+            "Should remove and then add a year field",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .bucket("data", 16)
+                    .year("year_field")
+                    .build(),
+            table.spec());
+    V2Assert.assertEquals(

Review Comment:
   Again, these two tests are essentially the same for me, right?  Please correct me if I missed something.
   



##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());
+
+    //  remove and add a field with TimeTransform(Years, Months, Days, Hours)
+    table.updateSpec().removeField("year_field_year").addField(year("year_field")).commit();

Review Comment:
   Do we need to add a case that remove `year_field_year` first and then add `year_field_year` after the deletion is successful?
   ```
   table.updateSpec().removeField("year_field_year").commit();
   assert……
   table.updateSpec().addField(year("year_field")).commit();
   
   ```



##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());

Review Comment:
   nit: `Should append a year partition field to the spec`?



-- 
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


[GitHub] [iceberg] zhangbutao commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1089690531


##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());
+    V2Assert.assertEquals(
+            "Should add a new id year",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .add(2, 1000, "data_bucket", Transforms.bucket(16))
+                    .add(3, 1001, "year_field_year", Transforms.year())
+                    .build(),
+            table.spec());
+
+    //  remove and add a field with TimeTransform(Years, Months, Days, Hours)
+    table.updateSpec().removeField("year_field_year").addField(year("year_field")).commit();
+
+    V1Assert.assertEquals(
+            "Should remove and then add a year field",
+            PartitionSpec.builderFor(table.schema())
+                    .withSpecId(1)
+                    .bucket("data", 16)
+                    .year("year_field")
+                    .build(),
+            table.spec());
+    V2Assert.assertEquals(

Review Comment:
   Oh, You mean `V1Assert.assertEquals` and `V2Assert.assertEquals`? I just follow other tests pattern and i think there maybe have some difference bettewn v2 and v1 formation, but i have not make more research.



-- 
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


[GitHub] [iceberg] zhangbutao commented on a diff in pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on code in PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#discussion_r1089692433


##########
core/src/test/java/org/apache/iceberg/TestTableUpdatePartitionSpec.java:
##########
@@ -187,6 +188,53 @@ public void testRemoveAndAddField() {
     Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testAddAfterRemoveTimeField() {
+    table.updateSpec().addField(year("year_field")).commit();
+
+    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16)
+            .year("year_field")
+            .build();
+
+    Assert.assertEquals(
+            "Should have same transform class: org.apache.iceberg.transforms.Years",
+            newSpec.fields().get(1).transform().getClass().getName(),
+            table.spec().fields().get(1).transform().getClass().getName());
+
+    V1Assert.assertEquals("Should add a new id year",  newSpec, table.spec());

Review Comment:
   Fixed. thx



-- 
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


[GitHub] [iceberg] zhangbutao commented on pull request #6482: API: Fix inconsistent TimeTransform Type

Posted by "zhangbutao (via GitHub)" <gi...@apache.org>.
zhangbutao commented on PR #6482:
URL: https://github.com/apache/iceberg/pull/6482#issuecomment-1413976249

   > Sorry for being late to the party here, I was traveling the last few days. I would be in favor of #6653 if that also solves your problem.
   
   Never mind :). Yes, https://github.com/apache/iceberg/pull/6653 can also fix this issue.  Feel free to close this PR if not needed. Thank you.  @Fokko 


-- 
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