morningman commented on code in PR #52591:
URL: https://github.com/apache/doris/pull/52591#discussion_r2211536292
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -1193,8 +1220,8 @@ private void sortReplicasByBackendCount(List<Replica>
replicas,
}
private void markReplicasForDropping(List<Replica> replicas, int
replicasToDrop,
- Map<Long, Long> tableBeToReplicaNumMap,
- Map<Long, Long> partitionBeToReplicaNumMap) {
+ Map<Long, Long> tableBeToReplicaNumMap,
Review Comment:
You need to change your IDEA's reformat logic to only reformat the line you
changed
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -1116,6 +1124,20 @@ public void replayCreateTable(String dbName, String
tblName) {
}
}
+ @Override
+ public void renameTable(String dbName, String oldTableName, String
newTableName) throws DdlException {
+ makeSureInitialized();
+ if (metadataOps == null) {
+ throw new DdlException("Rename table is not supported for catalog:
" + getName());
+ }
+ try {
+ metadataOps.renameTable(dbName, oldTableName, newTableName);
Review Comment:
No edit log?
##########
regression-test/suites/external_table_p0/iceberg/iceberg_schema_change_ddl.groovy:
##########
@@ -0,0 +1,210 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("iceberg_schema_change_ddl",
"p0,external,doris,external_docker,external_docker_doris") {
Review Comment:
Add more cases cross with branch tag
##########
fe/fe-core/src/main/java/org/apache/doris/persist/AddColumnLog.java:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.persist;
+
+import org.apache.doris.analysis.ColumnPosition;
+import org.apache.doris.catalog.Column;
+
+import com.google.gson.annotations.SerializedName;
+
+public class AddColumnLog extends AlterExternalTableSchemaLog {
Review Comment:
We don't need this info. Just use ExternalObjectLog.createForRefreshTable is
ok
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -516,6 +540,143 @@ public void dropBranchImpl(ExternalTable dorisTable,
DropBranchInfo branchInfo)
}
}
+ private void addOneColumn(UpdateSchema updateSchema, Column column) throws
UserException {
+ if (!column.isAllowNull()) {
+ throw new UserException("can't add a non-nullable column to an
Iceberg table");
+ }
+ org.apache.iceberg.types.Type dorisType =
IcebergUtils.dorisTypeToIcebergType(column.getType());
+ Literal<?> defaultValue =
IcebergUtils.parseIcebergLiteral(column.getDefaultValue(), dorisType);
+ updateSchema.addColumn(column.getName(), dorisType,
column.getComment(), defaultValue);
+ }
+
+ private void applyPosition(UpdateSchema updateSchema, ColumnPosition
position, String columnName) {
+ if (position.isFirst()) {
+ updateSchema.moveFirst(columnName);
+ } else {
+ updateSchema.moveAfter(columnName, position.getLastCol());
+ }
+ }
+
+ private void refreshTable(ExternalTable dorisTable) {
+ Optional<ExternalDatabase<?>> db =
dorisCatalog.getDbForReplay(dorisTable.getRemoteDbName());
+ if (db.isPresent()) {
+ Optional<?> tbl =
db.get().getTableForReplay(dorisTable.getRemoteName());
+ if (tbl.isPresent()) {
+ Env.getCurrentEnv().getRefreshManager()
+ .refreshTableInternal(db.get(), (ExternalTable)
tbl.get(), System.currentTimeMillis());
+ }
+ }
+ }
+
+ @Override
+ public void addColumn(ExternalTable dorisTable, Column column,
ColumnPosition position)
+ throws UserException {
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ addOneColumn(updateSchema, column);
+ if (position != null) {
+ applyPosition(updateSchema, position, column.getName());
+ }
+ try {
+ preExecutionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to add column: " +
column.getName() + " to table: "
+ + icebergTable.name() + ", error message is: " +
e.getMessage(), e);
+ }
+ refreshTable(dorisTable);
+ }
+
+ @Override
+ public void addColumns(ExternalTable dorisTable, List<Column> columns)
throws UserException {
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ for (Column column : columns) {
+ addOneColumn(updateSchema, column);
+ }
+ try {
+ preExecutionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to add columns to table: " +
icebergTable.name()
+ + ", error message is: " + e.getMessage(), e);
+ }
+ refreshTable(dorisTable);
+ }
+
+ @Override
+ public void dropColumn(ExternalTable dorisTable, String columnName) throws
UserException {
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ updateSchema.deleteColumn(columnName);
+ try {
+ preExecutionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to drop column: " + columnName + "
from table: "
+ + icebergTable.name() + ", error message is: " +
e.getMessage(), e);
+ }
+ refreshTable(dorisTable);
+ }
+
+ @Override
+ public void renameColumn(ExternalTable dorisTable, String oldName, String
newName) throws UserException {
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ updateSchema.renameColumn(oldName, newName);
+ try {
+ preExecutionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to rename column: " + oldName + "
to " + newName
+ + " in table: " + icebergTable.name() + ", error message
is: " + e.getMessage(), e);
+ }
+ refreshTable(dorisTable);
+ }
+
+ @Override
+ public void updateColumn(ExternalTable dorisTable, Column column,
ColumnPosition position)
+ throws UserException {
+ org.apache.iceberg.types.Type icebergType =
IcebergUtils.dorisTypeToIcebergType(column.getType());
+ if (!icebergType.isPrimitiveType()) {
+ throw new UserException("Update column type to non-primitive type
is not supported: " + column.getType());
+ }
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ updateSchema.updateColumn(column.getName(),
icebergType.asPrimitiveType(), column.getComment());
+ if (column.isAllowNull()) {
+ // we can change a required column to optional, but not the other
way around
+ // because we don't know whether there is existing data with null
values.
+ updateSchema.makeColumnOptional(column.getName());
+ }
+ if (position != null) {
+ applyPosition(updateSchema, position, column.getName());
+ }
+ try {
+ preExecutionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to update column: " +
column.getName() + " in table: "
+ + icebergTable.name() + ", error message is: " +
e.getMessage(), e);
+ }
+ refreshTable(dorisTable);
+ }
+
+ @Override
+ public void reorderColumns(ExternalTable dorisTable, List<String>
newOrder) throws UserException {
+ if (newOrder == null || newOrder.isEmpty()) {
+ throw new UserException("Reorder column failed, new order is
empty.");
+ }
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ updateSchema.moveFirst(newOrder.get(0));
+ for (int i = 1; i < newOrder.size(); i++) {
+ updateSchema.moveAfter(newOrder.get(i), newOrder.get(i - 1));
+ }
+ try {
+ preExecutionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to reorder columns in table: " +
icebergTable.name()
+ + ", error message is: " + e.getMessage(), e);
+ }
+ refreshTable(dorisTable);
Review Comment:
We should add edit log here
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
