github-actions[bot] commented on code in PR #65451:
URL: https://github.com/apache/doris/pull/65451#discussion_r3557033826
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -773,41 +786,58 @@ Status TableReader::_parse_delete_predicates(const
SplitReadOptions& options) {
DORIS_CHECK(options.cache != nullptr);
Status create_status = Status::OK();
- _delete_rows = options.cache->get<DeleteRows>(desc.key, [&]() ->
DeleteRows* {
- auto delete_rows = std::make_unique<DeleteRows>();
-
- DeletionVectorReader dv_reader(_runtime_state, _scanner_profile,
*_scan_params, desc,
- _io_ctx.get());
- create_status = dv_reader.open();
- if (!create_status.ok()) [[unlikely]] {
- return nullptr;
- }
-
- size_t bytes_read = desc.size;
- std::vector<char> buffer(bytes_read);
- DBUG_EXECUTE_IF("TableReader.parse_deletion_vector.io_error", {
- create_status = Status::IOError("injected format v2 deletion
vector read failure");
- return nullptr;
- });
- DBUG_EXECUTE_IF("TableReader.parse_deletion_vector.should_stop", {
- create_status = Status::EndOfFile("stop read.");
- return nullptr;
- });
- create_status = dv_reader.read_at(desc.start_offset,
{buffer.data(), bytes_read});
- if (!create_status.ok()) [[unlikely]] {
- return nullptr;
- }
-
- const char* buf = buffer.data();
- SCOPED_TIMER(_profile.parse_delete_file_time);
- create_status = parse_deletion_vector(buf, bytes_read,
desc.format, delete_rows.get());
- if (!create_status.ok()) [[unlikely]] {
- return nullptr;
- }
Review Comment:
This leaves Iceberg V2's DV sentinel stale.
`IcebergTableReader::_init_delete_predicates()` still treats `_delete_rows !=
nullptr` as "a deletion vector was parsed" and uses that to clear
`position_delete_files`, but this PR now stores the parsed DV in
`_deletion_vector` instead. For a split like the existing
`IcebergTableReaderIgnoresPositionDeleteFilesWhenDeletionVectorPresent` test,
`TableReader::prepare_split()` sets `_deletion_vector`, `_delete_rows` remains
null, `_init_position_delete_rows()` then reads the position-delete file, and
`_append_delete_predicate()` emits both predicates. That removes rows from the
DV and the position-delete file, contradicting the current Iceberg V2
behavior/test that position deletes are ignored when a DV is present. Please
update the Iceberg V2 guard to key off `_deletion_vector` (or otherwise make
the mixed-source behavior intentional and update the test).
--
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]
