From 0c744ac3c7e6e633fae672dda0f9bfd3aaa4150c Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 27 May 2026 11:40:02 -0700 Subject: [PATCH 1/4] PERF: Skip header check in parquet reader The parquet reader previously required 3 reads to read the parquet footer: 1. A 4 byte read to check the header for the parquet magic bytes 2. An 8 byte read to read the footer length and footer parquet magic bytes 3. A varaible-length read for the footer metadata We don't really care about ensuring that the header is valid. For high-latency storage, it's not worth the extra read. --- .../io/parquet/io_utils/parquet_io_utils.cpp | 8 +++---- cpp/tests/io/parquet_reader_test.cpp | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/cpp/src/io/parquet/io_utils/parquet_io_utils.cpp b/cpp/src/io/parquet/io_utils/parquet_io_utils.cpp index e762a0ae2f3a..6f3316581365 100644 --- a/cpp/src/io/parquet/io_utils/parquet_io_utils.cpp +++ b/cpp/src/io/parquet/io_utils/parquet_io_utils.cpp @@ -37,12 +37,10 @@ std::unique_ptr fetch_footer_to_host(cudf::io::dat constexpr auto ender_len = sizeof(file_ender_s); size_t const len = datasource.size(); - auto header_buffer = datasource.host_read(0, header_len); - auto const header = reinterpret_cast(header_buffer->data()); - auto ender_buffer = datasource.host_read(len - ender_len, ender_len); - auto const ender = reinterpret_cast(ender_buffer->data()); CUDF_EXPECTS(len > header_len + ender_len, "Incorrect data source"); - CUDF_EXPECTS(header->magic == detail::parquet_magic, "Corrupted header"); + + auto ender_buffer = datasource.host_read(len - ender_len, ender_len); + auto const ender = reinterpret_cast(ender_buffer->data()); CUDF_EXPECTS(ender->magic == detail::parquet_magic, "Corrupted footer"); CUDF_EXPECTS(ender->footer_len != 0 && ender->footer_len <= (len - header_len - ender_len), "Incorrect footer length"); diff --git a/cpp/tests/io/parquet_reader_test.cpp b/cpp/tests/io/parquet_reader_test.cpp index 9da69f9dad74..d4a9eeae7d1a 100644 --- a/cpp/tests/io/parquet_reader_test.cpp +++ b/cpp/tests/io/parquet_reader_test.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -4258,6 +4259,26 @@ TEST_F(ParquetReaderTest, LateBindSourceInfo) CUDF_TEST_EXPECT_TABLES_EQUAL(result.tbl->view(), expected->view()); } +TEST_F(ParquetReaderTest, InvalidFooterMagic) +{ + auto const expected = create_random_fixed_table(4, 4, false); + + auto const filepath = temp_env->get_temp_filepath("InvalidFooterMagic.parquet"); + cudf::io::write_parquet( + cudf::io::parquet_writer_options::builder(cudf::io::sink_info{filepath}, *expected)); + + std::fstream file(filepath, std::ios::in | std::ios::out | std::ios::binary); + ASSERT_TRUE(file.is_open()); + constexpr std::array bad_magic{'B', 'A', 'D', '!'}; + file.seekp(-static_cast(bad_magic.size()), std::ios::end); + file.write(bad_magic.data(), bad_magic.size()); + file.close(); + + auto const read_opts = + cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath}).build(); + EXPECT_THROW(cudf::io::read_parquet(read_opts), cudf::logic_error); +} + TEST_F(ParquetReaderTest, DecimalTypeOption) { auto const data = std::vector{1000, 2000, 3000, 4000, 5000}; From 269a7b9fc65994effd792a7ad33c3ce57320de81 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Thu, 28 May 2026 09:44:08 -0700 Subject: [PATCH 2/4] One more spot --- python/pylibcudf/pylibcudf/io/parquet.pyx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/python/pylibcudf/pylibcudf/io/parquet.pyx b/python/pylibcudf/pylibcudf/io/parquet.pyx index b7a064c4607e..4c90de8c2f0a 100644 --- a/python/pylibcudf/pylibcudf/io/parquet.pyx +++ b/python/pylibcudf/pylibcudf/io/parquet.pyx @@ -81,7 +81,9 @@ cdef vector[cpp_FileMetaData] _build_parquet_metadatas( size_t num_sources, ) except *: cdef vector[cpp_FileMetaData] c_metadatas + cdef vector[cpp_FileMetaData*] metadata_ptrs cdef object metadata + cdef size_t i if parquet_metadatas is None: return c_metadatas @@ -90,15 +92,22 @@ cdef vector[cpp_FileMetaData] _build_parquet_metadatas( raise TypeError( "parquet_metadatas must contain only FileMetaData objects" ) - c_metadatas.push_back((metadata).c_obj) + metadata_ptrs.push_back(&(metadata).c_obj) - if c_metadatas.size() != num_sources: + if metadata_ptrs.size() != num_sources: raise ValueError( - f"Length of 'parquet_metadatas' ({c_metadatas.size()}) " + f"Length of 'parquet_metadatas' ({metadata_ptrs.size()}) " f"must match the number of input sources " f"({num_sources})" ) + c_metadatas.reserve(metadata_ptrs.size()) + with nogil: + # This copies the (potentially large) metadata object. We don't + # want to hold the GIL for that. + for i in range(metadata_ptrs.size()): + c_metadatas.push_back(dereference(metadata_ptrs[i])) + return c_metadatas From edbd0e321f8a4626e56cbacc9cf5523e6f9d9ac7 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Thu, 28 May 2026 14:14:24 -0700 Subject: [PATCH 3/4] Revert "One more spot" This reverts commit 269a7b9fc65994effd792a7ad33c3ce57320de81. --- python/pylibcudf/pylibcudf/io/parquet.pyx | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/python/pylibcudf/pylibcudf/io/parquet.pyx b/python/pylibcudf/pylibcudf/io/parquet.pyx index 4c90de8c2f0a..b7a064c4607e 100644 --- a/python/pylibcudf/pylibcudf/io/parquet.pyx +++ b/python/pylibcudf/pylibcudf/io/parquet.pyx @@ -81,9 +81,7 @@ cdef vector[cpp_FileMetaData] _build_parquet_metadatas( size_t num_sources, ) except *: cdef vector[cpp_FileMetaData] c_metadatas - cdef vector[cpp_FileMetaData*] metadata_ptrs cdef object metadata - cdef size_t i if parquet_metadatas is None: return c_metadatas @@ -92,22 +90,15 @@ cdef vector[cpp_FileMetaData] _build_parquet_metadatas( raise TypeError( "parquet_metadatas must contain only FileMetaData objects" ) - metadata_ptrs.push_back(&(metadata).c_obj) + c_metadatas.push_back((metadata).c_obj) - if metadata_ptrs.size() != num_sources: + if c_metadatas.size() != num_sources: raise ValueError( - f"Length of 'parquet_metadatas' ({metadata_ptrs.size()}) " + f"Length of 'parquet_metadatas' ({c_metadatas.size()}) " f"must match the number of input sources " f"({num_sources})" ) - c_metadatas.reserve(metadata_ptrs.size()) - with nogil: - # This copies the (potentially large) metadata object. We don't - # want to hold the GIL for that. - for i in range(metadata_ptrs.size()): - c_metadatas.push_back(dereference(metadata_ptrs[i])) - return c_metadatas From 1b7f6a0ba1badd874e227b52d0b9d1d2248e480c Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 29 May 2026 07:04:30 -0700 Subject: [PATCH 4/4] avoid filesystem in test --- cpp/tests/io/parquet_reader_test.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cpp/tests/io/parquet_reader_test.cpp b/cpp/tests/io/parquet_reader_test.cpp index d4a9eeae7d1a..7caa41d7b186 100644 --- a/cpp/tests/io/parquet_reader_test.cpp +++ b/cpp/tests/io/parquet_reader_test.cpp @@ -29,7 +29,6 @@ #include #include -#include #include #include #include @@ -4263,19 +4262,20 @@ TEST_F(ParquetReaderTest, InvalidFooterMagic) { auto const expected = create_random_fixed_table(4, 4, false); - auto const filepath = temp_env->get_temp_filepath("InvalidFooterMagic.parquet"); + std::vector buffer; cudf::io::write_parquet( - cudf::io::parquet_writer_options::builder(cudf::io::sink_info{filepath}, *expected)); + cudf::io::parquet_writer_options::builder(cudf::io::sink_info{&buffer}, *expected)); - std::fstream file(filepath, std::ios::in | std::ios::out | std::ios::binary); - ASSERT_TRUE(file.is_open()); constexpr std::array bad_magic{'B', 'A', 'D', '!'}; - file.seekp(-static_cast(bad_magic.size()), std::ios::end); - file.write(bad_magic.data(), bad_magic.size()); - file.close(); + ASSERT_GE(buffer.size(), bad_magic.size()); + for (size_t i = 0; i < bad_magic.size(); ++i) { + buffer[buffer.size() - bad_magic.size() + i] = bad_magic[i]; + } - auto const read_opts = - cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath}).build(); + auto const read_opts = cudf::io::parquet_reader_options::builder( + cudf::io::source_info{cudf::host_span{ + reinterpret_cast(buffer.data()), buffer.size()}}) + .build(); EXPECT_THROW(cudf::io::read_parquet(read_opts), cudf::logic_error); }