Gabriel39 commented on PR #65135:
URL: https://github.com/apache/doris/pull/65135#issuecomment-4912478523

   I found two issues in the new position_deletes reader path:
   
   1. `IcebergPositionDeleteSysTableV2Reader::_append_deletion_vector_block` 
never reports EOF.
   
   In the V2 DV path, after advancing `_next_dv_position`, the function 
currently leaves `*eof = false`. Once the iterator reaches 
`_dv_positions.end()`, the next scanner loop can keep getting `read_rows = 0` 
with `eof = false`, which may hang or spin on empty blocks for Iceberg deletion 
vectors (`content = 3`). The V1 reader sets EOF from the iterator state, and V2 
should do the same:
   
   ```cpp
   *eof = *_next_dv_position == _dv_positions.end();
   ```
   
   A regression case with `enable_file_scanner_v2 = true` and a real deletion 
vector would cover this.
   
   2. The V2 position delete reader hardcodes Parquet.
   
   `IcebergPositionDeleteSysTableV2Reader::_init_position_delete_reader` 
initializes `PositionDeleteFileTableReader` with `.format = 
format::FileFormat::PARQUET`. V1 handles both Parquet and ORC based on 
`_delete_file_desc->file_format`, and Iceberg position delete files can be ORC. 
With scanner V2 enabled, an ORC position delete file will be read through the 
wrong format path or fail. Please map the delete file descriptor format into 
the V2 reader config, or explicitly reject unsupported formats with a clear 
error and test coverage.
   


-- 
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]

Reply via email to