github-actions[bot] commented on code in PR #65329:
URL: https://github.com/apache/doris/pull/65329#discussion_r3575453284
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -6359,7 +6476,7 @@ public AlterTableOp
visitRenamePartitionClause(RenamePartitionClauseContext ctx)
@Override
public AlterTableOp visitRenameColumnClause(RenameColumnClauseContext ctx)
{
- return new RenameColumnOp(ctx.name.getText(), ctx.newName.getText());
+ return new RenameColumnOp(ColumnPath.of(visitQualifiedName(ctx.name)),
ctx.newName.getText());
Review Comment:
`newName` still uses the raw token text for quoted rename targets. Even if
the source path normalization is fixed, a rename target like `` `New``Metric`
`` reaches `RenameColumnOp` as the literal backquoted token because this branch
reads `ctx.newName.getText()`. The default column-name validator rejects those
backticks, and with unicode-name support the raw quoted string is forwarded to
`updateSchema.renameColumn`, so Iceberg would see quoting characters instead of
the intended ``New`Metric`` field name. Please normalize `ctx.newName` with the
same identifier-unquoting helper used for path components.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -894,18 +1028,92 @@ public void modifyColumn(ExternalTable dorisTable,
Column column, ColumnPosition
refreshTable(dorisTable, updateTime);
}
+ @Override
+ public void modifyColumn(ExternalTable dorisTable, ColumnPath columnPath,
Column column, ColumnPosition position,
+ long updateTime) throws UserException {
+ if (!columnPath.isNested()) {
+ modifyColumn(dorisTable, column, position, updateTime);
+ return;
+ }
+
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ ResolvedColumnPath resolvedPath =
resolveColumnPath(icebergTable.schema(), columnPath, "modify");
+ NestedField currentCol = resolvedPath.getField();
+
+ validateCommonColumnInfo(column);
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+
+ if (column.getType().isComplexType()) {
+ validateForModifyComplexColumn(column, currentCol,
columnPath.getFullPath());
+ applyComplexTypeChange(updateSchema, resolvedPath.getFullPath(),
currentCol.type(), column.getType());
Review Comment:
This complex MODIFY path can still create case-insensitive duplicate struct
fields. `ADD COLUMN info.metric` and `RENAME ... TO metric` now call the
sibling collision guard, but `MODIFY COLUMN info STRUCT<Metric:...,
metric:...>` reaches this branch and then `applyStructChange` appends extra
fields with `updateSchema.addColumn(path, newField.getName(), ...)` without the
same guard. `ColumnType.checkSupportIcebergSchemaChangeForComplexType` only
tracks exact names, so a Spark schema with `Info.Metric` can add `metric`,
leaving Doris with two fields that collapse to the same visible name. Please
run the same case-insensitive sibling/new-field collision check before applying
struct additions, including nested structs under list elements and map values.
--
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]
