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/11/21 19:21:27 UTC

[GitHub] [iceberg] Fokko opened a new pull request, #6242: API: Restore the type of the identity transform

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

   This caused some regression for the Iceberg 1.1.0 release:
   
   ```
   2022-11-21T12:05:46.6549795Z [ERROR] io.trino.plugin.iceberg.TestIcebergSystemTables.testManifestsTable  Time elapsed: 0.701 s  <<< FAILURE!
   2022-11-21T12:05:46.6550853Z java.lang.AssertionError:
   2022-11-21T12:05:46.6551986Z [Rows for query [SELECT added_data_files_count, existing_rows_count, added_rows_count, deleted_data_files_count, deleted_rows_count, partitions FROM test_schema."test_table$manifests"]]
   2022-11-21T12:05:46.6553075Z Expecting:
   2022-11-21T12:05:46.6553593Z   <(2, 0, 3, 0, 0, [[false, false, 18148, 18149]]), (2, 0, 3, 0, 0, [[false, false, 18147, 18148]])>
   2022-11-21T12:05:46.6553980Z to contain exactly in any order:
   2022-11-21T12:05:46.6554557Z   <[(2, 0, 3, 0, 0, [[false, false, 2019-09-08, 2019-09-09]]),
   2022-11-21T12:05:46.6554992Z     (2, 0, 3, 0, 0, [[false, false, 2019-09-09, 2019-09-10]])]>
   2022-11-21T12:05:46.6555273Z elements not found:
   2022-11-21T12:05:46.6555804Z   <(2, 0, 3, 0, 0, [[false, false, 2019-09-08, 2019-09-09]]), (2, 0, 3, 0, 0, [[false, false, 2019-09-09, 2019-09-10]])>
   2022-11-21T12:05:46.6556132Z and elements not expected:
   2022-11-21T12:05:46.6556488Z   <(2, 0, 3, 0, 0, [[false, false, 18148, 18149]]), (2, 0, 3, 0, 0, [[false, false, 18147, 18148]])>
   ```
   
   The system tables (manifests in this example above), would return the days since epoch instead of a date.


-- 
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 merged pull request #6242: API: Restore the type of the identity transform

Posted by GitBox <gi...@apache.org>.
Fokko merged PR #6242:
URL: https://github.com/apache/iceberg/pull/6242


-- 
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] rdblue commented on pull request #6242: API: Restore the type of the identity transform

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

   Good catch. Thanks, @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] nastra commented on a diff in pull request #6242: API: Restore the type of the identity transform

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


##########
api/src/main/java/org/apache/iceberg/transforms/Identity.java:
##########
@@ -105,6 +142,21 @@ public boolean isIdentity() {
     return true;
   }
 
+  @Override
+  public boolean equals(Object o) {

Review Comment:
   is this something that we would remove once we remove the deprecated code?



-- 
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 #6242: API: Restore the type of the identity transform

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


##########
api/src/main/java/org/apache/iceberg/transforms/Identity.java:
##########
@@ -105,6 +142,21 @@ public boolean isIdentity() {
     return true;
   }
 
+  @Override
+  public boolean equals(Object o) {

Review Comment:
   Good call, just added it 👍🏻 



-- 
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 a diff in pull request #6242: API: Restore the type of the identity transform

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


##########
api/src/main/java/org/apache/iceberg/transforms/Identity.java:
##########
@@ -105,6 +142,21 @@ public boolean isIdentity() {
     return true;
   }
 
+  @Override
+  public boolean equals(Object o) {

Review Comment:
   I think it would be good to add a TODO here so that we don't forget to also remove `equals()` + `hashCode()` when removing the other deprecated methods



-- 
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 #6242: API: Restore the type of the identity transform

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


##########
api/src/main/java/org/apache/iceberg/transforms/Identity.java:
##########
@@ -105,6 +142,21 @@ public boolean isIdentity() {
     return true;
   }
 
+  @Override
+  public boolean equals(Object o) {

Review Comment:
   Yes, definitely. In the new situation, we only check for the memory reference (and that's okay since there is only one instance).



-- 
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 a diff in pull request #6242: API: Restore the type of the identity transform

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


##########
api/src/main/java/org/apache/iceberg/transforms/Identity.java:
##########
@@ -105,6 +142,21 @@ public boolean isIdentity() {
     return true;
   }
 
+  @Override
+  public boolean equals(Object o) {

Review Comment:
   I think it would be good to add a TODO here so that we don't forget to also remove `equals()` + `hashCode()`



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