github-actions[bot] commented on code in PR #66112:
URL: https://github.com/apache/doris/pull/66112#discussion_r3667030935
##########
be/src/exec/sink/writer/iceberg/viceberg_table_writer.cpp:
##########
@@ -485,9 +486,25 @@ Status VIcebergTableWriter::close(Status status) {
COUNTER_SET(_close_timer, _close_ns);
COUNTER_SET(_write_file_counter, _write_file_count);
}
+ if (!status.ok() || !result_status.ok()) {
+ _cleanup_closed_files();
+ } else {
+ _closed_files.clear();
Review Comment:
[P1] Keep data cleanup state through the outer merge close
This clears every retained data-file path as soon as the table writer itself
closes successfully, but `VIcebergMergeSink::close(Status::OK())` closes this
writer before the delete writer. If opening, writing, or closing a
position-delete/deletion-vector file then fails, the outer MERGE returns an
error after these handles are gone; `IcebergTransaction.rollback()` does not
remove insert/merge objects, so the successfully closed data files remain
orphaned. This is distinct from the existing late-duplicate thread: that path
enters the table close with a non-OK status, whereas this path starts OK and
fails only in the sibling delete close. Retain/transfer these handles until
both inner sinks succeed (and clean both sides on failure), with a delete-close
failpoint plus object-store assertion.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateTableCommand.java:
##########
@@ -99,6 +99,13 @@ public void run(ConnectContext ctx, StmtExecutor executor)
throws Exception {
LOG.debug("Nereids start to execute the ctas command, query id:
{}, tableName: {}",
ctx.queryId(), createTableInfo.getTableName());
}
+ LogicalPlan sinkQuery = null;
+ if (!createTableInfo.isIfNotExists()) {
Review Comment:
[P2] Preserve the existing-target diagnostic before sink prevalidation
For CTAS without `IF NOT EXISTS`, this now constructs the sink before asking
the catalog to create the table. A stable existing Paimon target therefore
fails here with `Load data to PaimonExternalCatalog is not supported`, instead
of the catalog's `TABLE ALREADY EXISTS` error, even though no write or
rollback-ownership decision is possible. The new tests cover the existing `IF
NOT EXISTS` no-op and a missing unsupported target, but not this branch.
Preserve the existing-object precedence (without treating a precheck as
ownership proof), and add that regression 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]
