loserwang1024 commented on code in PR #2189:
URL: https://github.com/apache/fluss/pull/2189#discussion_r2626700528


##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -322,24 +322,26 @@ public long createTable(
     }
 
     public void alterTableSchema(
-            TablePath tablePath, List<TableChange> schemaChanges, boolean 
ignoreIfNotExists)
+            TablePath tablePath,
+            List<TableChange> schemaChanges,
+            boolean ignoreIfNotExists,
+            @Nullable LakeCatalog lakeCatalog,
+            LakeCatalog.Context lakeCatalogContext)
             throws TableNotExistException, TableNotPartitionedException {
         try {
 
             TableInfo table = getTable(tablePath);
 
-            // TODO: remote this after lake enable table support schema 
evolution, track by
-            // https://github.com/apache/fluss/issues/2128
-            if (table.getTableConfig().isDataLakeEnabled()) {
-                throw new InvalidAlterTableException(
-                        "Schema evolution is currently not supported for 
tables with datalake enabled.");
-            }
-
             // validate the table column changes
             if (!schemaChanges.isEmpty()) {
                 Schema newSchema = SchemaUpdate.applySchemaChanges(table, 
schemaChanges);
-                // update the schema
-                zookeeperClient.registerSchema(tablePath, newSchema, 
table.getSchemaId() + 1);
+                // update the schema in Fluss (ZK) first - Fluss is the source 
of truth
+                if (!newSchema.equals(table.getSchema())) {

Review Comment:
   Yes, If no other add column is happened, it will handle it gracefully. 
However, as I mention before:
   
   Aassumed that:
   1. Add column A
   - zookeeperClient.registerSchema success
   - syncSchemaChangesToLake fails
   
   2. Add column B
   - zookeeperClient.registerSchema success
   - syncSchemaChangesToLake success
   
   3. ReAdd column A
   - SchemaUpdate.applySchemaChanges is Idempotent
   - syncSchemaChangesToLake success.
   
   Finally, Fluss Table is : column A, column B
   Finally, Lake Table is : column B, column A.



##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -322,24 +322,26 @@ public long createTable(
     }
 
     public void alterTableSchema(
-            TablePath tablePath, List<TableChange> schemaChanges, boolean 
ignoreIfNotExists)
+            TablePath tablePath,
+            List<TableChange> schemaChanges,
+            boolean ignoreIfNotExists,
+            @Nullable LakeCatalog lakeCatalog,
+            LakeCatalog.Context lakeCatalogContext)
             throws TableNotExistException, TableNotPartitionedException {
         try {
 
             TableInfo table = getTable(tablePath);
 
-            // TODO: remote this after lake enable table support schema 
evolution, track by
-            // https://github.com/apache/fluss/issues/2128
-            if (table.getTableConfig().isDataLakeEnabled()) {
-                throw new InvalidAlterTableException(
-                        "Schema evolution is currently not supported for 
tables with datalake enabled.");
-            }
-
             // validate the table column changes
             if (!schemaChanges.isEmpty()) {
                 Schema newSchema = SchemaUpdate.applySchemaChanges(table, 
schemaChanges);
-                // update the schema
-                zookeeperClient.registerSchema(tablePath, newSchema, 
table.getSchemaId() + 1);
+                // update the schema in Fluss (ZK) first - Fluss is the source 
of truth
+                if (!newSchema.equals(table.getSchema())) {

Review Comment:
   @buvb Yes, If no other add column is happened, it will handle it gracefully. 
However, as I mention before:
   
   Aassumed that:
   1. Add column A
   - zookeeperClient.registerSchema success
   - syncSchemaChangesToLake fails
   
   2. Add column B
   - zookeeperClient.registerSchema success
   - syncSchemaChangesToLake success
   
   3. ReAdd column A
   - SchemaUpdate.applySchemaChanges is Idempotent
   - syncSchemaChangesToLake success.
   
   Finally, Fluss Table is : column A, column B
   Finally, Lake Table is : column B, column A.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to