[GitHub] [iceberg] rdblue commented on a change in pull request #2096: Core: add schema id and schemas to table metadata

[GitBox <gi...@apache.org>]

rdblue commented on a change in pull request #2096:
URL: https://github.com/apache/iceberg/pull/2096#discussion_r569016246



##########
File path: api/src/main/java/org/apache/iceberg/Schema.java
##########
@@ -328,6 +329,24 @@ private Schema internalSelect(Collection<String> names, boolean caseSensitive) {
     return TypeUtil.select(this, selected);
   }
 
+  @Override
+  public boolean equals(Object other) {
+    if (this == other) {
+      return true;
+    } else if (!(other instanceof Schema)) {
+      return false;
+    }
+
+    Schema that = (Schema) other;
+    return struct.equals(that.struct) &&
+        Objects.equals(aliasToId, that.aliasToId);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(struct, aliasToId);
+  }

Review comment:
       I don't think that should use aliases. Aliases are populated when converting from a file schema to Iceberg, so that we can look up the file's column names from IDs and vice versa. That information is used by integration and not by Iceberg itself. The aliases are not serialized when writing a schema into Iceberg metadata.
   
   For the purposes of maintaining the schema list, we can ignore aliases and just use `schema.asStruct()`.




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



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