gavinchou commented on code in PR #11494:
URL: https://github.com/apache/doris/pull/11494#discussion_r937331773
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1851,4 +1840,8 @@ public void checkReplicaAllocation() throws UserException
{
}
}
}
+
+ public Boolean getUseLightSchemaChange() {
Review Comment:
Isn't it a property of OlapTable?
Also, naming it as `lightSchemaChangeEanbled()` seems better.
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -957,62 +937,97 @@ private boolean addColumnInternal(OlapTable olapTable,
Column newColumn, ColumnP
for (Map.Entry<Long, LinkedList<Column>> entry :
indexSchemaMap.entrySet()) {
modIndexSchema = entry.getValue();
boolean isBaseIdex = entry.getKey() == baseIndexId;
- checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet, isBaseIdex);
+ IntSupplier colUniqueIdSupplier =
colUniqueIdSupplierMap.get(entry.getKey());
+ int newColumnUniqueId = olapTable.getUseLightSchemaChange()
+ ? Column.COLUMN_UNIQUE_ID_INIT_VALUE :
colUniqueIdSupplier.getAsInt();
+ checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet,
+ isBaseIdex, newColumnUniqueId);
}
} else {
// 1. add to base table
modIndexSchema = indexSchemaMap.get(baseIndexId);
- checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet, true);
+ IntSupplier baseIndexColUniqueIdSupplier =
colUniqueIdSupplierMap.get(baseIndexId);
Review Comment:
Remove duplicated code, we don't have to check `olapTable.getUseLight()`
every time we generate a new `columnUniqueId` in a single code block.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/InternalDataSource.java:
##########
@@ -1703,11 +1703,6 @@ private void createOlapTable(Database db,
CreateTableStmt stmt) throws UserExcep
OlapTable olapTable = new OlapTable(tableId, tableName, baseSchema,
keysType, partitionInfo,
defaultDistributionInfo, indexes);
- for (Column column : baseSchema) {
Review Comment:
We need to ensure that this change won't break the original behavior of base
table.
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -957,62 +937,97 @@ private boolean addColumnInternal(OlapTable olapTable,
Column newColumn, ColumnP
for (Map.Entry<Long, LinkedList<Column>> entry :
indexSchemaMap.entrySet()) {
modIndexSchema = entry.getValue();
boolean isBaseIdex = entry.getKey() == baseIndexId;
- checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet, isBaseIdex);
+ IntSupplier colUniqueIdSupplier =
colUniqueIdSupplierMap.get(entry.getKey());
+ int newColumnUniqueId = olapTable.getUseLightSchemaChange()
+ ? Column.COLUMN_UNIQUE_ID_INIT_VALUE :
colUniqueIdSupplier.getAsInt();
+ checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet,
+ isBaseIdex, newColumnUniqueId);
}
} else {
// 1. add to base table
modIndexSchema = indexSchemaMap.get(baseIndexId);
- checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet, true);
+ IntSupplier baseIndexColUniqueIdSupplier =
colUniqueIdSupplierMap.get(baseIndexId);
+ int baseIndexNewColumnUniqueId =
olapTable.getUseLightSchemaChange()
+ ? Column.COLUMN_UNIQUE_ID_INIT_VALUE
+ : baseIndexColUniqueIdSupplier.getAsInt();
+ checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet,
+ true, baseIndexNewColumnUniqueId);
if (targetIndexId == -1L) {
- return ligthSchemaChange;
+ return lightSchemaChange;
}
// 2. add to rollup
- ligthSchemaChange = false;
+ lightSchemaChange = false;
modIndexSchema = indexSchemaMap.get(targetIndexId);
- checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet, false);
+ IntSupplier targetIndexColUniqueIdSupplier =
colUniqueIdSupplierMap.get(targetIndexId);
+ int rollUpNewColumnUniqueId =
olapTable.getUseLightSchemaChange()
+ ? Column.COLUMN_UNIQUE_ID_INIT_VALUE
+ : targetIndexColUniqueIdSupplier.getAsInt();
+ checkAndAddColumn(modIndexSchema, newColumn, columnPos,
newColNameSet,
+ false, rollUpNewColumnUniqueId);
}
} else if (KeysType.DUP_KEYS == olapTable.getKeysType()) {
if (targetIndexId == -1L) {
Review Comment:
Remove duplicated code, try this:
```
IntSupplier idSupplier = colUniqueIdSupplierMap.get(targetIndexId == -1 ?
baseIndexId : targetIndexId);
Int newId = olapTable.getUseLightSchemaChange() ?
Column.COLUMN_UNIQUE_ID_INIT_VALUE : idSupplier.getAsInt();
...
if target == -1
checkAndAddColumn(... newId)
else
checkAndAddColumn(... newId)
```
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
