Skip to content

Commit 1d3f63a

Browse files
SteNicholasSteNicholas
committed
test(parquet): allocate raw Arrow reader buffers from the default pool
Several Parquet tests open files with arrow::io::ReadableFile and parquet::arrow::OpenFile, passing a raw pointer to the fixture's GetArrowPool() adaptor. Pre-buffering is enabled by default, so an Arrow IO thread can drop the last reference to a read buffer after the fixture has been torn down. ~PoolBuffer() then frees into the destroyed pool and paimon-parquet-format-test crashes with SIGSEGV, as seen in VariantParquetTest. Use arrow::default_memory_pool() for these raw readers so the pool outlives any in-flight async read. Paimon's own read path is unaffected: ArrowInputStreamAdapter already keeps its pool alive for returned buffers.
1 parent 6fa3c81 commit 1d3f63a

3 files changed

Lines changed: 48 additions & 35 deletions

File tree

‎src/paimon/format/parquet/parquet_file_batch_reader_test.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2056,11 +2056,12 @@ TEST_F(ParquetFileBatchReaderTest, TestDictionaryPassthroughSkipsFallbackToPlain
20562056
// its presence cannot be the signal; the data pages are what say the column went plain. Both
20572057
// are asserted so the test fails loudly if the fixture stops producing a mixed chunk rather
20582058
// than quietly passing for the wrong reason.
2059-
auto metadata_file = arrow::io::ReadableFile::Open(file_path, pool_.get());
2059+
auto metadata_file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
20602060
ASSERT_TRUE(metadata_file.ok());
20612061
std::unique_ptr<::parquet::arrow::FileReader> metadata_reader;
2062-
ASSERT_TRUE(
2063-
::parquet::arrow::OpenFile(metadata_file.ValueOrDie(), pool_.get(), &metadata_reader).ok());
2062+
ASSERT_TRUE(::parquet::arrow::OpenFile(metadata_file.ValueOrDie(), arrow::default_memory_pool(),
2063+
&metadata_reader)
2064+
.ok());
20642065
std::unique_ptr<::parquet::ColumnChunkMetaData> column_chunk =
20652066
metadata_reader->parquet_reader()->metadata()->RowGroup(0)->ColumnChunk(0);
20662067
ASSERT_TRUE(column_chunk->has_dictionary_page());
@@ -2151,11 +2152,12 @@ TEST_F(ParquetFileBatchReaderTest, TestDictionaryPassthroughRequiresEveryRowGrou
21512152

21522153
// Pin the fixture: without this the read assertion below would also pass on a file whose
21532154
// first row group was never dictionary-encoded in the first place.
2154-
auto metadata_file = arrow::io::ReadableFile::Open(file_path, pool_.get());
2155+
auto metadata_file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
21552156
ASSERT_TRUE(metadata_file.ok());
21562157
std::unique_ptr<::parquet::arrow::FileReader> metadata_reader;
2157-
ASSERT_TRUE(
2158-
::parquet::arrow::OpenFile(metadata_file.ValueOrDie(), pool_.get(), &metadata_reader).ok());
2158+
ASSERT_TRUE(::parquet::arrow::OpenFile(metadata_file.ValueOrDie(), arrow::default_memory_pool(),
2159+
&metadata_reader)
2160+
.ok());
21592161
std::shared_ptr<::parquet::FileMetaData> metadata =
21602162
metadata_reader->parquet_reader()->metadata();
21612163
ASSERT_EQ(2, metadata->num_row_groups());

‎src/paimon/format/parquet/parquet_format_writer_test.cpp‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,13 @@ class ParquetFormatWriterTest : public ::testing::Test {
142142

143143
void CheckResult(const std::string& file_path, int32_t row_count,
144144
int32_t row_group_count) const {
145-
auto file = arrow::io::ReadableFile::Open(file_path, arrow_pool_.get());
146-
ASSERT_TRUE(file.ok());
145+
// Use the process-wide pool: pre-buffered reads may release their buffers on Arrow IO
146+
// threads after this fixture's pool is destroyed.
147+
auto file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
148+
ASSERT_TRUE(file.ok()) << file.status().ToString();
147149
std::unique_ptr<::parquet::arrow::FileReader> reader;
148-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
150+
auto status =
151+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
149152
ASSERT_TRUE(status.ok()) << status.ToString();
150153
const ::parquet::FileMetaData* metadata = reader->parquet_reader()->metadata().get();
151154
const ::parquet::SchemaDescriptor* schema = metadata->schema();
@@ -266,10 +269,11 @@ class ParquetFormatWriterTest : public ::testing::Test {
266269

267270
void CheckEncodedResult(const std::string& file_path, int32_t row_count,
268271
bool null_in_dictionary) const {
269-
auto file = arrow::io::ReadableFile::Open(file_path, arrow_pool_.get());
270-
ASSERT_TRUE(file.ok());
272+
auto file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
273+
ASSERT_TRUE(file.ok()) << file.status().ToString();
271274
std::unique_ptr<::parquet::arrow::FileReader> reader;
272-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
275+
auto status =
276+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
273277
ASSERT_TRUE(status.ok()) << status.ToString();
274278
const ::parquet::FileMetaData* metadata = reader->parquet_reader()->metadata().get();
275279
ASSERT_EQ(metadata->num_rows(), row_count);
@@ -636,10 +640,11 @@ TEST_F(ParquetFormatWriterTest, TestWriteDictionaryChangingAcrossBatches) {
636640
ASSERT_OK(out->Flush());
637641
ASSERT_OK(out->Close());
638642

639-
auto file = arrow::io::ReadableFile::Open(file_path, arrow_pool_.get());
640-
ASSERT_TRUE(file.ok());
643+
auto file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
644+
ASSERT_TRUE(file.ok()) << file.status().ToString();
641645
std::unique_ptr<::parquet::arrow::FileReader> reader;
642-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
646+
auto status =
647+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
643648
ASSERT_TRUE(status.ok()) << status.ToString();
644649
const ::parquet::FileMetaData* metadata = reader->parquet_reader()->metadata().get();
645650
ASSERT_EQ(3 * kBatchRows, metadata->num_rows());
@@ -708,10 +713,11 @@ TEST_F(ParquetFormatWriterTest, TestWriteDictionaryWithNullRows) {
708713
ASSERT_OK(out->Flush());
709714
ASSERT_OK(out->Close());
710715

711-
auto file = arrow::io::ReadableFile::Open(file_path, arrow_pool_.get());
712-
ASSERT_TRUE(file.ok());
716+
auto file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
717+
ASSERT_TRUE(file.ok()) << file.status().ToString();
713718
std::unique_ptr<::parquet::arrow::FileReader> reader;
714-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
719+
auto status =
720+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
715721
ASSERT_TRUE(status.ok()) << status.ToString();
716722
const ::parquet::FileMetaData* metadata = reader->parquet_reader()->metadata().get();
717723
ASSERT_EQ(6, metadata->num_rows());
@@ -766,10 +772,11 @@ TEST_F(ParquetFormatWriterTest, TestWriteDictionaryWithDuplicateValues) {
766772
ASSERT_OK(out->Flush());
767773
ASSERT_OK(out->Close());
768774

769-
auto file = arrow::io::ReadableFile::Open(file_path, arrow_pool_.get());
770-
ASSERT_TRUE(file.ok());
775+
auto file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
776+
ASSERT_TRUE(file.ok()) << file.status().ToString();
771777
std::unique_ptr<::parquet::arrow::FileReader> reader;
772-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
778+
auto status =
779+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
773780
ASSERT_TRUE(status.ok()) << status.ToString();
774781
const ::parquet::FileMetaData* metadata = reader->parquet_reader()->metadata().get();
775782
ASSERT_EQ(4, metadata->num_rows());
@@ -889,10 +896,11 @@ TEST_F(ParquetFormatWriterTest, TestWriteDictionaryOfBinaryColumn) {
889896
ASSERT_OK(out->Flush());
890897
ASSERT_OK(out->Close());
891898

892-
auto file = arrow::io::ReadableFile::Open(file_path, arrow_pool_.get());
893-
ASSERT_TRUE(file.ok());
899+
auto file = arrow::io::ReadableFile::Open(file_path, arrow::default_memory_pool());
900+
ASSERT_TRUE(file.ok()) << file.status().ToString();
894901
std::unique_ptr<::parquet::arrow::FileReader> reader;
895-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
902+
auto status =
903+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
896904
ASSERT_TRUE(status.ok()) << status.ToString();
897905
const ::parquet::FileMetaData* metadata = reader->parquet_reader()->metadata().get();
898906
ASSERT_EQ(4, metadata->num_rows());

‎src/paimon/format/parquet/variant_parquet_test.cpp‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -545,10 +545,11 @@ TEST_F(VariantParquetTest, PhysicalLayoutMatchesJava) {
545545
// reader is required because these parquet-level properties (repetition, physical types,
546546
// field ids, the absence of a logical-type annotation) are not visible in the Arrow schema
547547
// surfaced by the paimon reader.
548-
auto file = arrow::io::ReadableFile::Open(file_path_, arrow_pool_.get());
549-
ASSERT_TRUE(file.ok());
548+
auto file = arrow::io::ReadableFile::Open(file_path_, arrow::default_memory_pool());
549+
ASSERT_TRUE(file.ok()) << file.status().ToString();
550550
std::unique_ptr<::parquet::arrow::FileReader> reader;
551-
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
551+
auto status =
552+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &reader);
552553
ASSERT_TRUE(status.ok()) << status.ToString();
553554
const ::parquet::SchemaDescriptor* schema = reader->parquet_reader()->metadata()->schema();
554555
ASSERT_EQ(schema->num_columns(), 3);
@@ -591,12 +592,14 @@ TEST_F(VariantParquetTest, WriteAndReadRoundTrip) {
591592

592593
{
593594
// Sanity-check the raw file through the plain parquet-arrow reader: the struct child
594-
// arrays must align with the logical rows.
595-
auto file = arrow::io::ReadableFile::Open(file_path_, arrow_pool_.get());
596-
ASSERT_TRUE(file.ok());
595+
// arrays must align with the logical rows. Use the process-wide pool: pre-buffered reads
596+
// may release their buffers on Arrow IO threads after this fixture's pool is destroyed.
597+
auto file = arrow::io::ReadableFile::Open(file_path_, arrow::default_memory_pool());
598+
ASSERT_TRUE(file.ok()) << file.status().ToString();
597599
std::unique_ptr<::parquet::arrow::FileReader> raw_reader;
598600
ASSERT_TRUE(
599-
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &raw_reader).ok());
601+
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow::default_memory_pool(), &raw_reader)
602+
.ok());
600603
std::shared_ptr<arrow::Table> table;
601604
ASSERT_TRUE(raw_reader->ReadTable(&table).ok());
602605
auto raw_variant = checked_pointer_cast<arrow::StructArray>(table->column(1)->chunk(0));
@@ -712,11 +715,11 @@ TEST_F(VariantParquetTest, ShreddedWriteAndReadRoundTrip) {
712715
}
713716

714717
{
715-
auto file = arrow::io::ReadableFile::Open(file_path_, arrow_pool_.get());
716-
ASSERT_TRUE(file.ok());
718+
auto file = arrow::io::ReadableFile::Open(file_path_, arrow::default_memory_pool());
719+
ASSERT_TRUE(file.ok()) << file.status().ToString();
717720
std::unique_ptr<::parquet::arrow::FileReader> reader;
718-
auto status =
719-
::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), &reader);
721+
auto status = ::parquet::arrow::OpenFile(file.ValueOrDie(),
722+
arrow::default_memory_pool(), &reader);
720723
ASSERT_TRUE(status.ok()) << status.ToString();
721724
const auto* root = reader->parquet_reader()->metadata()->schema()->group_node();
722725
ASSERT_EQ(root->field_count(), 2);

0 commit comments

Comments
 (0)