Gabriel39 commented on code in PR #66627:
URL: https://github.com/apache/doris/pull/66627#discussion_r3751756847
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergNestedColumnEvolution.java:
##########
@@ -88,11 +88,38 @@ public static void addColumn(Table table,
ConnectorColumnPath path, IcebergColum
/** Drops the nested field at {@code path}; its parent must resolve to a
struct that contains the leaf. */
public static void dropColumn(Table table, ConnectorColumnPath path) {
ResolvedColumnPath resolvedPath =
validateNestedStructFieldPath(table.schema(), path, "drop");
+ validateNotUsedByOldPartitionSpec(table, resolvedPath);
UpdateSchema updateSchema = table.updateSchema();
updateSchema.deleteColumn(resolvedPath.getFullPath());
updateSchema.commit();
}
+ static void dropTopLevelColumn(Table table, String columnName) {
+ ResolvedColumnPath resolvedPath = resolveColumnPath(
+ table.schema(), ConnectorColumnPath.of(columnName), "drop");
+ validateNotUsedByOldPartitionSpec(table, resolvedPath);
+ UpdateSchema updateSchema = table.updateSchema();
+ updateSchema.deleteColumn(resolvedPath.getFullPath());
+ updateSchema.commit();
+ }
+
+ private static void validateNotUsedByOldPartitionSpec(Table table,
ResolvedColumnPath columnPath) {
+ int currentSpecId = table.spec().specId();
+ Set<Integer> droppedFieldIds = TypeUtil.indexById(
+ Types.StructType.of(columnPath.getField())).keySet();
+ // Historical specs resolve partition types by source field ID against
the current schema, so deleting
+ // a referenced source field or any ancestor leaves those specs
unreadable even after the field is
+ // removed from the current spec. Index the full subtree because
deleting a struct also deletes its fields.
+ boolean usedByOldSpec = table.specs().values().stream()
+ .filter(spec -> spec.specId() != currentSpecId)
+ .flatMap(spec -> spec.fields().stream())
Review Comment:
Fixed in d8931cb2696. The drop now emits a transient add/remove
partition-spec update so Iceberg REST requires the original last-assigned
partition ID, and retries the complete refresh/resolve/validate/commit attempt
on CommitFailedException. A deterministic rebase test injects a concurrent
non-default spec and verifies the retry rejects the drop before a second commit.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergNestedColumnEvolution.java:
##########
@@ -88,11 +88,38 @@ public static void addColumn(Table table,
ConnectorColumnPath path, IcebergColum
/** Drops the nested field at {@code path}; its parent must resolve to a
struct that contains the leaf. */
public static void dropColumn(Table table, ConnectorColumnPath path) {
ResolvedColumnPath resolvedPath =
validateNestedStructFieldPath(table.schema(), path, "drop");
+ validateNotUsedByOldPartitionSpec(table, resolvedPath);
UpdateSchema updateSchema = table.updateSchema();
updateSchema.deleteColumn(resolvedPath.getFullPath());
updateSchema.commit();
}
+ static void dropTopLevelColumn(Table table, String columnName) {
+ ResolvedColumnPath resolvedPath = resolveColumnPath(
+ table.schema(), ConnectorColumnPath.of(columnName), "drop");
+ validateNotUsedByOldPartitionSpec(table, resolvedPath);
+ UpdateSchema updateSchema = table.updateSchema();
+ updateSchema.deleteColumn(resolvedPath.getFullPath());
+ updateSchema.commit();
+ }
+
+ private static void validateNotUsedByOldPartitionSpec(Table table,
ResolvedColumnPath columnPath) {
+ int currentSpecId = table.spec().specId();
+ Set<Integer> droppedFieldIds = TypeUtil.indexById(
+ Types.StructType.of(columnPath.getField())).keySet();
+ // Historical specs resolve partition types by source field ID against
the current schema, so deleting
+ // a referenced source field or any ancestor leaves those specs
unreadable even after the field is
+ // removed from the current spec. Index the full subtree because
deleting a struct also deletes its fields.
+ boolean usedByOldSpec = table.specs().values().stream()
+ .filter(spec -> spec.specId() != currentSpecId)
+ .flatMap(spec -> spec.fields().stream())
+ .anyMatch(field -> droppedFieldIds.contains(field.sourceId()));
Review Comment:
Fixed in d8931cb2696. Retained-spec validation now includes matching void
fields in the current format-v1 spec. The regression removes the older spec,
verifies the current void field is the only retained reference, rejects the
drop, and confirms the position_deletes metadata-table schema remains
constructible.
--
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]
