[GitHub] [iceberg] Initial-neko commented on a change in pull request #3614: Flink: Fix ALTER for tables with primary key fields

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

Initial-neko commented on a change in pull request #3614:
URL: https://github.com/apache/iceberg/pull/3614#discussion_r767391329



##########
File path: flink/v1.13/flink/src/main/java/org/apache/iceberg/flink/FlinkCatalog.java
##########
@@ -422,7 +440,7 @@ public void alterTable(ObjectPath tablePath, CatalogBaseTable newTable, boolean
 
     // For current Flink Catalog API, support for adding/removing/renaming columns cannot be done by comparing
     // CatalogTable instances, unless the Flink schema contains Iceberg column IDs.
-    if (!table.getSchema().equals(newTable.getSchema())) {
+    if (!schemaCompare(table.getUnresolvedSchema(), newTable.getUnresolvedSchema())) {

Review comment:
       **schema**
   
   Schema of a table or view.
   A schema represents the schema part of a CREATE TABLE (schema) WITH (options) DDL statement in SQL. It defines columns of different kind, constraints, time attributes, and watermark strategies. It is possible to reference objects (such as functions or types) across different catalogs.
   **This class is used in the API and catalogs to define an unresolved schema that will be translated to ResolvedSchema. Some methods of this class perform basic validation, however, the main validation happens during the resolution. Thus, an unresolved schema can be incomplete and might be enriched or merged with a different schema at a later stage.**
   Since an instance of this class is unresolved, it should not be directly persisted. The toString() shows only a summary of the contained objects.
   
   **getUnresolvedSchema**
   
   Returns the schema of the table or view.
   **The schema can reference objects from other catalogs and will be resolved and validated by the framework when accessing the table or view.**
   
   **getSchema**
   
   This method returns the deprecated TableSchema class. **The old class was a hybrid of resolved and unresolved schema information**. It has been replaced by the new Schema which is always unresolved and will be resolved by the framework later.
   
   I'm not particularly clear about the meaning of unresolved, but according to the above comments, the framework will perform additional parsing on the unresolved schema. After testing, both getschema and getunresolved schema have no problem in the 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