OIiveirra commented on code in PR #69408:
URL: https://github.com/apache/doris/pull/69408#discussion_r4351771070
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -967,17 +966,198 @@ public void dropColumn(ExternalTable dorisTable,
ColumnPath columnPath, long upd
dropColumn(dorisTable, columnPath.getTopLevelName(), updateTime);
return;
}
+ SessionContext context = SessionContext.current();
try (IcebergExternalMetaCache.WritableTableLease lease =
acquireWritableTable(dorisTable)) {
- Table icebergTable = lease.getTable();
- ResolvedColumnPath resolvedPath =
validateNestedStructFieldPath(icebergTable.schema(), columnPath, "drop");
+ dropColumnSafely(lease, dorisTable, columnPath, context);
+ }
+ refreshAfterColumnDrop(dorisTable, updateTime);
+ }
- UpdateSchema updateSchema = icebergTable.updateSchema();
- updateSchema.deleteColumn(resolvedPath.getFullPath());
- commitTableUpdate(lease, updateSchema::commit,
- "Failed to drop nested column: " +
columnPath.getFullPath() + " from table: "
- + icebergTable.name());
+ private void dropColumnSafely(IcebergExternalMetaCache.WritableTableLease
lease,
+ ExternalTable dorisTable, ColumnPath path, SessionContext context)
+ throws UserException {
+ try {
+ // A refresh can perform authenticated catalog IO just like the
eventual commit. Keep target
+ // validation, refresh and commit inside the one catalog
generation retained by this lease.
+ lease.getAuthenticator().execute(() -> {
+ commitColumnDrop(lease, dorisTable, path, context);
+ return null;
+ });
+ } catch (UserException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new UserException("Failed to drop column: " +
path.getFullPath() + ", error message is: "
+ + ExceptionUtils.getRootCauseMessage(e), e);
+ }
+ }
+
+ private void refreshAfterColumnDrop(ExternalTable dorisTable, long
updateTime) throws UserException {
+ try {
+ refreshTable(dorisTable, updateTime);
+ } catch (RuntimeException e) {
+ throw new UserException("Iceberg column DROP committed, but Doris
metadata refresh failed for "
+ + dorisTable.getRemoteDbName() + "." +
dorisTable.getRemoteName()
+ + "; refresh the table before another operation: " +
e.getMessage(), e);
+ }
+ }
+
+ private void commitColumnDrop(IcebergExternalMetaCache.WritableTableLease
lease,
+ ExternalTable dorisTable, ColumnPath path, SessionContext context)
throws UserException {
+ Table table = lease.getTable();
+ if (!path.isNested()) {
+ validateRowLineageColumnMutation(table, path.getTopLevelName(),
"drop");
+ }
+ TableOperations operations = ((HasTableOperations) table).operations();
+ TableMetadata loaded = operations.current();
+ if (loaded == null) {
+ throw new UserException("Cannot drop column from an unloaded
Iceberg table: " + table.name());
+ }
+ ResolvedColumnPath loadedPath = resolveDropPath(loaded.schema(), path);
+ Set<Integer> loadedSubtreeIds = fieldSubtreeIds(loadedPath);
+
+ TableMetadata base = operations.refresh();
+ ResolvedColumnPath resolvedPath = validatePinnedDropIdentity(
+ table.name(), path, loaded, loadedSubtreeIds, base);
+ validateNotUsedByRetainedPartitionSpec(base, resolvedPath);
+
+ // Stage the standard SDK update against exactly the validated base.
Recapturing operations.current()
+ // here could instead delete a same-name replacement published on a
shared handle after refresh.
+ CapturingTableOperations schemaCapture = new
CapturingTableOperations(operations, base);
+ Table stagedTable = new BaseTable(schemaCapture, table.name());
+ if (!path.isNested()) {
+ validateRowLineageColumnMutation(stagedTable,
path.getTopLevelName(), "drop");
+ }
+
stagedTable.updateSchema().deleteColumn(resolvedPath.getFullPath()).commit();
+ TableMetadata schemaUpdate = schemaCapture.committed();
+ if (isRestTableOperations(operations)) {
+ // No retained spec references the target, so a new SDK partition
reference must allocate a field ID.
Review Comment:
Confirmed and fixed in `f2d8c4748179f20a05256a88fab4e66204455f2e`. The real
SDK/HTTP reproduction failed on the previous implementation because the DROP
unexpectedly committed after the stale writer restored the expired source with
an unchanged counter.
The existing Iceberg 1.11.0 REST requirements cannot assert the entire
retained-spec set or metadata version. Numeric partition-ID coverage is also
insufficient because upgraded v1 history can contain different source mappings
for the same ID. The minimal safe fix explicitly rejects REST DROP whenever
partition field IDs have ever been allocated, before schema staging or
submission. This includes currently unpartitioned tables after history cleanup
and unrelated ordinary/nested columns; the accepted compatibility restriction
is stated in the PR body and release note.
Never-partitioned REST tables keep the explicit transaction/counter
requirement. Direct catalogs retain full-metadata CAS. Other DDL,
authentication, dependencies and protocol are unchanged; no extra durable
metadata mutation or unsafe fallback was added.
Validation: 373 focused FE tests passed (21 real HTTP tests), including
v2/v3 stale writers, upgraded-v1 ID aliases, expired/nested history and
direct-CAS controls; Checkstyle passed. The final standard ASAN BE+FE build and
both complete native suites (`iceberg_schema_change_ddl` and
`test_iceberg_delete_unpartitioned_evolution`) passed with final
artifact/process identity checks and no failed, fatal or skipped suites. The
evolution suite includes the new ordinary-column rejection assertion. Existing
result snapshots were unchanged.
--
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]
