github-actions[bot] commented on code in PR #69392:
URL: https://github.com/apache/doris/pull/69392#discussion_r4355478721
##########
be/test/exec/sink/viceberg_delete_sink_test.cpp:
##########
@@ -533,36 +564,120 @@ TEST_F(VIcebergDeleteSinkTest,
TestValidateDeletionVectorContentSize) {
ASSERT_TRUE(unsupported_status.is<ErrorCode::NOT_IMPLEMENTED_ERROR>()) <<
unsupported_status;
}
+TEST_F(VIcebergDeleteSinkTest, TestWritePositionDeletesCommitsTableLocation) {
+ std::filesystem::path temp_dir = std::filesystem::temp_directory_path() /
+ ("iceberg_delete_sink_test_" +
generate_uuid_string());
+ ASSERT_TRUE(std::filesystem::create_directories(temp_dir));
+
+ // Write locally, but record the table's original URI in the commit
metadata.
+ const std::string table_location = "oss://bucket/warehouse/table/data/";
+ const std::string data_file_path = table_location + "file1.parquet";
+ TDataSink t_data_sink = build_local_delete_sink(temp_dir.string(), 2);
+ t_data_sink.iceberg_delete_sink.__set_table_location(table_location);
+ RuntimeState state;
+ VExprContextSPtrs output_exprs;
+ auto sink = std::make_shared<VIcebergDeleteSink>(t_data_sink,
output_exprs, nullptr, nullptr);
+ ObjectPool pool;
+ ASSERT_TRUE(sink->init_properties(&pool).ok());
+ sink->_state = &state;
+
+ std::map<std::string, IcebergFileDeletion> file_deletions;
+ auto [it, inserted] =
+ file_deletions.emplace(data_file_path, IcebergFileDeletion(1,
"[\"p=1\"]"));
+ ASSERT_TRUE(inserted);
+ it->second.rows_to_delete.add(uint32_t {10});
+ it->second.rows_to_delete.add(uint32_t {20});
+
+ const auto status = sink->_write_position_delete_files(file_deletions);
Review Comment:
[P2] Initialize the Arrow memory pool before the Parquet write
This new test reaches the real Parquet writer, but its fixture never
initializes `ExecEnv::_arrow_memory_pool`: it defaults to null, and
`run_all_tests.cpp` only calls `init_mem_tracker()`, which does not set it.
`VIcebergDeleteFileWriter` enables dictionary encoding, and
`VParquetWriter::_parse_properties()` explicitly forwards that null pool into
`WriterProperties`. In Arrow 24, the [dictionary
encoder](https://github.com/apache/arrow/blob/apache-arrow-24.0.0/cpp/src/parquet/encoder.cc#L428-L432)
retains it in `buffered_indices_`; the first value's insertion reaches the
[STL
allocator](https://github.com/apache/arrow/blob/apache-arrow-24.0.0/cpp/src/arrow/stl_allocator.h#L49-L61),
which calls `pool_->Allocate` without a null fallback. Thus running this test
in isolation crashes before it can verify the commit-path assertions. Save,
initialize, and restore the pool as `VParquetWriterTest::SetUp/TearDown`
already does.
--
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]
