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/16 22:51:19 UTC

[GitHub] [iceberg] islamismailov opened a new issue, #6204: Allow dropping a column used by old SortOrders

islamismailov opened a new issue, #6204:
URL: https://github.com/apache/iceberg/issues/6204

   ### Apache Iceberg version
   
   1.0.0 (latest release)
   
   ### Query engine
   
   Spark
   
   ### Please describe the bug 🐞
   
   Here's my first go at a [PR for this](https://gist.github.com/islamismailov/49b3e2a9952f5fb86bab522d9aec7019), I did it on top of a 0.9 branch. I am willing to create a proper PR if that approach seems sound.
   
   Repro:
   
   ```
   CREATE TABLE temp.sort_order_col_ref_drop_test (a INT, b INT);
   ALTER TABLE temp.sort_order_col_ref_drop_test WRITE ORDERED BY a;
   ALTER TABLE temp.sort_order_col_ref_drop_test WRITE ORDERED BY b;
   ALTER TABLE temp.sort_order_col_ref_drop_test DROP column a; -- FAIL
   ```
   
   Exception stack trace:
   
   ```
   java.lang.NullPointerException: Cannot find source column: 1
     at org.apache.iceberg.relocated.com.google.common.base.Preconditions.checkNotNull(Preconditions.java:953)
     at org.apache.iceberg.SortOrder$Builder.addSortField(SortOrder.java:241)
     at org.apache.iceberg.TableMetadata.updateSortOrderSchema(TableMetadata.java:829)
     at org.apache.iceberg.TableMetadata.lambda$updateSchema$1(TableMetadata.java:460)
     at org.apache.iceberg.relocated.com.google.common.collect.Lists$TransformingRandomAccessList$1.transform(Lists.java:612)
     at org.apache.iceberg.relocated.com.google.common.collect.TransformedIterator.next(TransformedIterator.java:47)
     at org.apache.iceberg.TableMetadata.indexSortOrders(TableMetadata.java:891)
     at org.apache.iceberg.TableMetadata.<init>(TableMetadata.java:282)
     at org.apache.iceberg.TableMetadata.updateSchema(TableMetadata.java:464)
     at org.apache.iceberg.SchemaUpdate.commit(SchemaUpdate.java:369)
     at org.apache.iceberg.spark.SparkCatalog.commitChanges(SparkCatalog.java:448)
     at org.apache.iceberg.spark.SparkCatalog.alterTable(SparkCatalog.java:241)
     at ...
   ```


-- 
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.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 issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
Fokko commented on issue #6204:
URL: https://github.com/apache/iceberg/issues/6204#issuecomment-1323246249

   Thanks for double checking 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 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 issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
Fokko commented on issue #6204:
URL: https://github.com/apache/iceberg/issues/6204#issuecomment-1318969998

   Feel free to open up a PR 👍🏻 


-- 
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] islamismailov commented on issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
islamismailov commented on issue #6204:
URL: https://github.com/apache/iceberg/issues/6204#issuecomment-1319124286

   Added a PR https://github.com/apache/iceberg/pull/6211


-- 
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 issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
Fokko commented on issue #6204:
URL: https://github.com/apache/iceberg/issues/6204#issuecomment-1318656478

   @islamismailov I'm just seeing your gist. I think it looks good, except one copy-paste:
   ```
   +      if (defaultSortOrderId == null) {
   +        throw new IllegalArgumentException("Default sort order ID unset");
   +      }
   +
   +      if (defaultSortOrderId == 0) { // This should be 0 instead of null
   +        throw new IllegalArgumentException("Default sort order ID 0 is reserved for unsorted order");
   +      }
   +
   ```


-- 
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 issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
Fokko closed issue #6204: Allow dropping a column used by old SortOrders
URL: https://github.com/apache/iceberg/issues/6204


-- 
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] islamismailov commented on issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
islamismailov commented on issue #6204:
URL: https://github.com/apache/iceberg/issues/6204#issuecomment-1318941894

   Ok thank you, yes I noticed that problem just before sending it out. Can I send a PR? Or do I need to get on an approved list of committers first?


-- 
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 issue #6204: Allow dropping a column used by old SortOrders

Posted by GitBox <gi...@apache.org>.
Fokko commented on issue #6204:
URL: https://github.com/apache/iceberg/issues/6204#issuecomment-1318209724

   Thanks for opening this PR @islamismailov. We had something similar for the partition-spec: https://github.com/apache/iceberg/issues/5676 What we did there was only look up the columns of the current partition spec. I think we can do something similar for the sort-order. Would you be interested in fixing 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 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