OIiveirra commented on code in PR #69408:
URL: https://github.com/apache/doris/pull/69408#discussion_r4360119342
##########
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:
The PR has been narrowed to the original direct-catalog requirement in
`552536c9fe61fbc0f787aa3be0ca6aff2c119f94`.
The REST low-ID/stale-writer issue remains valid and is now tracked
separately in #69544; narrowing this PR does not fix that issue. REST keeps
branch-4.2's ordinary SDK table-update endpoint, loaded-table credentials and
cache/handle behavior. The transaction-specific adapter and the numeric
partition-history restriction have been removed from the final diff, and the
affected REST regression suites are restored to the fixed base. Three real-HTTP
compatibility cases verify that ordinary REST drops do not require the
transaction endpoint, that unrelated columns on partitioned tables remain
droppable, and that loaded-table credentials are retained.
For catalogs with complete metadata CAS, the DROP uses all retained specs,
pins UUID/subtree identity and stages against exactly the validated base before
one CAS commit. Low-ID restoration tests cover both currently partitioned and
now-unpartitioned tables. The metadata-file UUID reread also catches Hadoop's
known cached-version reuse after recreation, following the existing transaction
pattern; it is not claimed to replace Hadoop's underlying numeric-version
protocol with a full atomic UUID fence.
Validation: 370 focused FE tests and Checkstyle passed; the final standard
ASAN BE+FE build and all three complete native suites passed with stable
source/artifact/process identity, zero failed/fatal/skipped suites, and fresh
task state. The local ASAN process cap was raised from 8GiB to 32GiB after an
initial process-limit cancellation; assertions and source were unchanged.
Please assess the current direct-catalog scope separately from the explicitly
tracked REST follow-up.
--
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]
