github-actions[bot] commented on code in PR #65401:
URL: https://github.com/apache/doris/pull/65401#discussion_r3548953477


##########
be/test/format/table/iceberg/iceberg_delete_file_reader_helper_test.cpp:
##########
@@ -107,4 +166,67 @@ TEST(IcebergDeleteFileReaderHelperTest, 
ReadMixedEncodingParquetPositionDeleteFi
     EXPECT_EQ(it->second, expected_positions);
 }
 
+TEST(IcebergDeleteFileReaderHelperTest, 
ReadOptionalParquetPositionDeleteColumnsWithoutNulls) {
+    const auto delete_file_path =
+            
temp_parquet_path("iceberg_optional_position_delete_without_nulls.parquet");
+    write_position_delete_parquet(delete_file_path,
+                                  {std::string(kTargetDataFilePath),
+                                   std::string(kTargetDataFilePath), 
"s3://other/file.parquet"},
+                                  {3, 9, 11});
+
+    RuntimeProfile profile("test_profile");
+    RuntimeState runtime_state((TQueryOptions()), TQueryGlobals());
+    FileMetaCache meta_cache(1024);
+    IcebergDeleteFileIOContext io_context(&runtime_state);
+
+    TFileScanRangeParams scan_params;
+    scan_params.file_type = TFileType::FILE_LOCAL;
+    scan_params.format_type = TFileFormatType::FORMAT_PARQUET;
+
+    TIcebergDeleteFileDesc delete_file;
+    delete_file.path = delete_file_path;
+    delete_file.file_format = TFileFormatType::FORMAT_PARQUET;
+    delete_file.__isset.file_format = true;
+
+    CollectPositionDeleteVisitor visitor;
+    auto options =

Review Comment:
   The new tests cover `read_iceberg_position_delete_file()` directly, but this 
patch also changes the scan-reader paths that build and consume their own 
delete blocks: v1 `IcebergParquetReader::_read_position_delete_file` / 
`_gen_position_delete_file_range`, and format_v2 
`PositionDeleteRowsCollector::collect`. Existing format_v2 position-delete 
tests still write `file_path` and `pos` as required Arrow fields, so they do 
not exercise optional schemas or verify that an actual null propagates as an 
error through `prepare_split` / `get_block`. Please add reader-path coverage, 
at least for format_v2 and ideally the v1 reader too, using nullable 
`file_path`/`pos` delete files for both the no-null success case and the 
actual-null rejection 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]

Reply via email to