github-actions[bot] commented on code in PR #65329:
URL: https://github.com/apache/doris/pull/65329#discussion_r3565933180
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -802,6 +838,42 @@ public void addColumn(ExternalTable dorisTable, Column
column, ColumnPosition po
refreshTable(dorisTable, updateTime);
}
+ @Override
+ public void addColumn(ExternalTable dorisTable, ColumnPath columnPath,
Column column, ColumnPosition position,
+ long updateTime) throws UserException {
+ if (!columnPath.isNested()) {
+ addColumn(dorisTable, column, position, updateTime);
+ return;
+ }
+ validateCommonColumnInfo(column);
+ if (!column.isAllowNull()) {
+ throw new UserException("New nested field '" +
columnPath.getFullPath() + "' must be nullable");
+ }
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ ResolvedColumnPath parentPath =
resolveColumnPath(icebergTable.schema(), columnPath.getParentPath(), "add");
+ if (!parentPath.getType().isStructType()) {
+ throw new UserException("Parent column path '" +
columnPath.getParentPathString()
+ + "' is not a struct in Iceberg table: " +
icebergTable.name());
+ }
+
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ org.apache.iceberg.types.Type dorisType =
IcebergUtils.dorisTypeToIcebergType(column.getType());
+ Literal<?> defaultValue =
IcebergUtils.parseIcebergLiteral(column.getDefaultValue(), dorisType);
+ updateSchema.addColumn(parentPath.getFullPath(),
columnPath.getLeafName(), dorisType,
Review Comment:
Please reject case-insensitive sibling collisions before calling
`UpdateSchema` here. This path canonicalizes the parent, but it still passes
the raw requested leaf to Iceberg, so a Spark-created `Info STRUCT<Metric:
INT>` can accept `ALTER TABLE t ADD COLUMN info.metric STRING NULL` as a new
child named `metric` beside existing `Metric`. Doris resolves and displays
Iceberg fields case-insensitively/lower-cased, so after refresh those two
fields collapse to the same Doris-visible path. The nested rename path has the
same problem because it resolves the old path and then passes raw `newName` to
`renameColumn`. Please check the resolved parent struct with
`caseInsensitiveField(...)` for ADD and RENAME targets, and add a mixed-case
Spark/Doris regression for this case.
--
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]
