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 2019/11/12 01:05:13 UTC

[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #574: Add incompatible changes to UpdateSchema API

rdblue commented on a change in pull request #574: Add incompatible changes to UpdateSchema API
URL: https://github.com/apache/incubator-iceberg/pull/574#discussion_r344976641
 
 

 ##########
 File path: core/src/main/java/org/apache/iceberg/SchemaUpdate.java
 ##########
 @@ -340,26 +407,30 @@ public Type field(Types.NestedField field, Type fieldResult) {
     }
 
     @Override
-    public Type list(Types.ListType list, Type result) {
+    public Type list(Types.ListType list, Type elementResult) {
       // use field to apply updates
-      Type elementResult = field(list.fields().get(0), result);
-      if (elementResult == null) {
+      Types.NestedField elementField = list.fields().get(0);
+      Type elementType = field(elementField, elementResult);
+      if (elementType == null) {
         throw new IllegalArgumentException("Cannot delete element type from list: " + list);
       }
 
-      if (list.elementType() == elementResult) {
+      Types.NestedField elementUpdate = updates.get(elementField.fieldId());
+      boolean isElementOptional = elementUpdate != null ? elementUpdate.isOptional() : list.isElementOptional();
+
+      if (isElementOptional == elementField.isOptional() && list.elementType() == elementType) {
 
 Review comment:
   Because elements, keys, and values are fields, they have doc strings, but these aren't actually exposed in the type API because these are accessed as types instead of fields. For example, `list.elementType()`.
   
   There is no direct getter, like `list.elementDoc()`, and I don't think it would be useful to add those, since you can document the list's elements as part of the list. Does that make sense?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org