github-actions[bot] commented on code in PR #65329:
URL: https://github.com/apache/doris/pull/65329#discussion_r3607449269
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -1134,6 +1522,37 @@ private void validateCommonColumnInfo(Column column)
throws UserException {
if (column.isAutoInc()) {
throw new UserException("Can not specify auto incremental iceberg
table column");
}
+ try {
+ IcebergUtils.dorisTypeToIcebergType(column.getType());
Review Comment:
[P1] Validate mapped complex schemas without rejecting unchanged children
With `enable.mapping.varbinary=true`, an existing Iceberg UUID/BINARY/FIXED
child is exposed in Doris as `VARBINARY`. A valid change such as `MODIFY COLUMN
info.payload STRUCT<blob:VARBINARY,n:BIGINT>` now converts the entire
replacement type here before diffing leaves, but
`DorisTypeToIcebergType.atomic` has no VARBINARY mapping, so the unchanged
`blob` makes the change to `n` fail. The recursive comparisons similarly
hardcode both mapping flags to `false`, so enabled TIMESTAMPTZ mappings are
compared against DATETIMEV2. Please validate/apply changed leaves against their
actual Iceberg types (or thread the catalog mapping settings through this
conversion) and cover complex schemas with unchanged mapped siblings.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -947,18 +1084,101 @@ 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()) {
+ modifyTopLevelColumn(dorisTable, column, position, updateTime,
false);
+ return;
+ }
+
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ ResolvedColumnPath resolvedPath =
resolveColumnPath(icebergTable.schema(), columnPath, "modify");
+ NestedField currentCol = resolvedPath.getField();
+ if (position != null) {
+ validatePositionTarget(icebergTable.schema(),
resolvedPath.getColumnPath(), "modify");
+ }
+
+ validateNestedModifyColumnMetadata(column, resolvedPath.getFullPath());
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+
+ if (column.getType().isComplexType()) {
+ validateForModifyComplexColumn(column, currentCol,
columnPath.getFullPath());
+ applyComplexTypeChange(updateSchema, resolvedPath.getFullPath(),
currentCol.type(), column.getType(),
+ false);
+ if (!Objects.equals(currentCol.doc(), column.getComment())) {
+ updateSchema.updateColumnDoc(resolvedPath.getFullPath(),
column.getComment());
+ }
+ } else {
+ validateForModifyColumn(column, currentCol,
columnPath.getFullPath());
+ Type icebergType =
IcebergUtils.dorisTypeToIcebergType(column.getType());
Review Comment:
[P1] Preserve the existing Iceberg primitive for mapped no-op MODIFY
For a Spark-created nested UUID field, Doris exposes the type as `STRING`
when varbinary mapping is disabled, so an advertised reorder/nullability
operation must repeat that visible type, for example `MODIFY COLUMN info.id
STRING NULL AFTER other`. This line converts it back to Iceberg `string` and
calls `updateColumn` on the UUID field; Iceberg only accepts the same type or a
valid promotion, so the operation fails before applying the requested
nullability/position. The same identity loss affects BINARY/FIXED and zoned
timestamps when their mapping flags are off, while mapped VARBINARY is rejected
even earlier. When the requested Doris type equals the mapping-aware view of
`currentCol.type()`, please retain the actual Iceberg primitive and update only
the requested metadata, with Spark-created coverage under both mapping settings.
--
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]
