Gabriel39 commented on code in PR #66348:
URL: https://github.com/apache/doris/pull/66348#discussion_r3710446917
##########
be/src/exec/sink/viceberg_delete_sink.cpp:
##########
@@ -283,13 +283,17 @@ Status VIcebergDeleteSink::close(Status close_status) {
_delete_file_count);
if (_state != nullptr) {
- for (const auto& commit_data : _commit_data_list) {
- _state->add_iceberg_commit_datas(commit_data);
+ for (auto& commit_data : _commit_data_list) {
+ Status report_status =
_state->add_iceberg_commit_datas(std::move(commit_data));
+ if (!report_status.ok()) {
+ _cleanup_created_files();
+ return report_status;
+ }
}
}
if (!_defer_file_cleanup_until_outer_close) {
- _created_files.clear();
+ _transfer_created_files_to_report_cleanup();
Review Comment:
Fixed in e7195899e1. I extracted the coordinator report-ACK validation into
a shared helper and now call it from both VIcebergTableWriter::open() and the
standalone VIcebergDeleteSink::open() before sink initialization or file
creation. This closes the new-BE/old-FE ownership gap for direct DELETE while
preserving the existing merge path gate. I also added
VIcebergDeleteSinkTest.RejectsCoordinatorWithoutExternalFileReportAck; the
focused ASAN suite (all VIcebergDeleteSinkTest cases plus the table-writer
compatibility test) passed 15/15 before the conflict-only rebase, whose
range-diff did not change this C++ patch. I am leaving the thread unresolved
for reviewer verification.
--
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]
