You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/02 14:05:31 UTC
[GitHub] [doris] morningman commented on a diff in pull request #10136: [Feature] Lightweight schema change of add/drop column
morningman commented on code in PR #10136:
URL: https://github.com/apache/doris/pull/10136#discussion_r912365661
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java:
##########
@@ -4922,4 +4916,190 @@ private static void addTableComment(Table table, StringBuilder sb) {
sb.append("\nCOMMENT '").append(table.getComment(true)).append("'");
}
}
+
+ // the invoker should keep table's write lock
+ public void modifyTableAddOrDropColumns(Database db, OlapTable olapTable,
Review Comment:
Better move this method to `SchemaChangeHandler`, don't make Catalog.java too big
##########
fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java:
##########
@@ -222,6 +222,8 @@ public class OperationType {
// policy 310-320
public static final short OP_CREATE_POLICY = 310;
public static final short OP_DROP_POLICY = 311;
+ //schema change for add and drop columns 320-329
+ public static final short OP_MODIFY_TABLE_ADD_OR_DROP_COLUMNS = 320;
Review Comment:
better put it after `public static final short OP_MODIFY_TABLE_ENGINE = 127;`
and use `128`
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java:
##########
@@ -4922,4 +4916,190 @@ private static void addTableComment(Table table, StringBuilder sb) {
sb.append("\nCOMMENT '").append(table.getComment(true)).append("'");
}
}
+
+ // the invoker should keep table's write lock
+ public void modifyTableAddOrDropColumns(Database db, OlapTable olapTable,
+ Map<Long, LinkedList<Column>> indexSchemaMap,
+ List<Index> indexes, long jobId, boolean isReplay) throws DdlException {
+
+ LOG.debug("indexSchemaMap:{}, indexes:{}", indexSchemaMap, indexes);
+ if (olapTable.getState() == OlapTableState.ROLLUP) {
+ throw new DdlException("Table[" + olapTable.getName() + "]'s is doing ROLLUP job");
+ }
+
+ // for now table's state can only be NORMAL
+ Preconditions.checkState(olapTable.getState() == OlapTableState.NORMAL, olapTable.getState().name());
+
+ // for bitmapIndex
+ boolean hasIndexChange = false;
+ Set<Index> newSet = new HashSet<>(indexes);
+ Set<Index> oriSet = new HashSet<>(olapTable.getIndexes());
+ if (!newSet.equals(oriSet)) {
+ hasIndexChange = true;
+ }
+
+ // begin checking each table
+ // ATTN: DO NOT change any meta in this loop
+ Map<Long, List<Column>> changedIndexIdToSchema = Maps.newHashMap();
+ for (Long alterIndexId : indexSchemaMap.keySet()) {
+ // Must get all columns including invisible columns.
+ // Because in alter process, all columns must be considered.
+ List<Column> alterSchema = indexSchemaMap.get(alterIndexId);
+
+ LOG.debug("index[{}] is changed. start checking...", alterIndexId);
+ // 1. check order: a) has key; b) value after key
+ boolean meetValue = false;
+ boolean hasKey = false;
+ for (Column column : alterSchema) {
+ if (column.isKey() && meetValue) {
+ throw new DdlException(
+ "Invalid column order. value should be after key. index[" + olapTable.getIndexNameById(
+ alterIndexId) + "]");
+ }
+ if (!column.isKey()) {
+ meetValue = true;
+ } else {
+ hasKey = true;
+ }
+ }
+ if (!hasKey) {
+ throw new DdlException("No key column left. index[" + olapTable.getIndexNameById(alterIndexId) + "]");
+ }
+
+ // 2. check partition key
+ PartitionInfo partitionInfo = olapTable.getPartitionInfo();
+ if (partitionInfo.getType() == PartitionType.RANGE || partitionInfo.getType() == PartitionType.LIST) {
+ List<Column> partitionColumns = partitionInfo.getPartitionColumns();
+ for (Column partitionCol : partitionColumns) {
+ boolean found = false;
+ for (Column alterColumn : alterSchema) {
+ if (alterColumn.nameEquals(partitionCol.getName(), true)) {
+ found = true;
+ break;
+ }
+ } // end for alterColumns
+
+ if (!found && alterIndexId == olapTable.getBaseIndexId()) {
+ // 2.1 partition column cannot be deleted.
+ throw new DdlException(
+ "Partition column[" + partitionCol.getName() + "] cannot be dropped. index["
+ + olapTable.getIndexNameById(alterIndexId) + "]");
+ }
+ } // end for partitionColumns
+ }
+
+ // 3. check distribution key:
+ DistributionInfo distributionInfo = olapTable.getDefaultDistributionInfo();
+ if (distributionInfo.getType() == DistributionInfoType.HASH) {
+ List<Column> distributionColumns = ((HashDistributionInfo) distributionInfo).getDistributionColumns();
+ for (Column distributionCol : distributionColumns) {
+ boolean found = false;
+ for (Column alterColumn : alterSchema) {
+ if (alterColumn.nameEquals(distributionCol.getName(), true)) {
+ found = true;
+ break;
+ }
+ } // end for alterColumns
+ if (!found && alterIndexId == olapTable.getBaseIndexId()) {
+ // 2.2 distribution column cannot be deleted.
+ throw new DdlException(
+ "Distribution column[" + distributionCol.getName() + "] cannot be dropped. index["
+ + olapTable.getIndexNameById(alterIndexId) + "]");
+ }
+ } // end for distributionCols
+ }
+
+ // 5. store the changed columns for edit log
+ changedIndexIdToSchema.put(alterIndexId, alterSchema);
+
+ LOG.debug("schema change[{}-{}-{}] check pass.", db.getId(), olapTable.getId(), alterIndexId);
+ } // end for indices
+
+ if (changedIndexIdToSchema.isEmpty() && !hasIndexChange) {
+ throw new DdlException("Nothing is changed. please check your alter stmt.");
+ }
+
+ //update base index schema
+ long baseIndexId = olapTable.getBaseIndexId();
+ List<Long> indexIds = new ArrayList<Long>();
+ indexIds.add(baseIndexId);
+ indexIds.addAll(olapTable.getIndexIdListExceptBaseIndex());
+ for (int i = 0; i < indexIds.size(); i++) {
+ List<Column> indexSchema = indexSchemaMap.get(indexIds.get(i));
+ MaterializedIndexMeta currentIndexMeta = olapTable.getIndexMetaByIndexId(indexIds.get(i));
+ currentIndexMeta.setSchema(indexSchema);
+
+ int currentSchemaVersion = currentIndexMeta.getSchemaVersion();
+ int newSchemaVersion = currentSchemaVersion + 1;
+ currentIndexMeta.setSchemaVersion(newSchemaVersion);
+ // generate schema hash for new index has to generate a new schema hash not equal to current schema hash
+ int currentSchemaHash = currentIndexMeta.getSchemaHash();
+ int newSchemaHash = Util.generateSchemaHash();
+ while (currentSchemaHash == newSchemaHash) {
+ newSchemaHash = Util.generateSchemaHash();
+ }
+ currentIndexMeta.setSchemaHash(newSchemaHash);
+ }
+ olapTable.setIndexes(indexes);
+ olapTable.rebuildFullSchema();
+
+ //update max column unique id
+ int maxColUniqueId = olapTable.getMaxColUniqueId();
+ for (Column column : indexSchemaMap.get(olapTable.getBaseIndexId())) {
+ if (column.getUniqueId() > maxColUniqueId) {
+ maxColUniqueId = column.getUniqueId();
+ }
+ }
+ olapTable.setMaxColUniqueId(maxColUniqueId);
+
+ if (!isReplay) {
+ TableAddOrDropColumnsInfo info = new TableAddOrDropColumnsInfo(db.getId(), olapTable.getId(),
+ indexSchemaMap, indexes, jobId);
+ LOG.debug("logModifyTableAddOrDropColumns info:{}", info);
+ editLog.logModifyTableAddOrDropColumns(info);
+ }
+
+ //for compatibility, we need create a finished state schema change job v2
+
+ SchemaChangeJobV2 schemaChangeJob = new SchemaChangeJobV2(jobId, db.getId(), olapTable.getId(),
+ olapTable.getName(), 1000);
+ schemaChangeJob.setJobState(AlterJobV2.JobState.FINISHED);
+ schemaChangeJob.setFinishedTimeMs(System.currentTimeMillis());
Review Comment:
I think we should save the read `createTime` and `finishTime` of this schema change job in `TableAddOrDropColumnsInfo`.(Maybe we can just use one timestamp for both create and finish time).
Otherwise, it will make user confused because after FE restart, the finish time of the schema change job will be changed.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org