loserwang1024 commented on code in PR #2189:
URL: https://github.com/apache/fluss/pull/2189#discussion_r2625453318
##########
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:
My adive is:
1. check that fluss can apply schema change.
SchemaUpdate.applySchemaChanges(table, schemaChanges)
2. Check whether the schema can be apply to lake.
3. zookeeperClient.registerSchema(tablePath, newSchema, table.getSchemaId()
+ 1). This can be rarely happen because only zookeeper.
4. syncSchemaChangesToLake.
Also a problem left is what if zookeeperClient.registerSchema success but
syncSchemaChangesToLake fails? A assumed 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.
I advice each time before zookeeperClient.registerSchema, check whether the
column number is same:
1. Add column A
- zookeeperClient.registerSchema success
- syncSchemaChangesToLake fails
2. Add column B
- Columns number is not same, just refuse it.
@wuchong , WDYT?
--
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]
