This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit f9c46259d731c9ef7d54d8d54d1f298e0c743d0e Author: Gabriel <[email protected]> AuthorDate: Wed Sep 16 18:03:57 2026 +0800 [fix](be) Backport Parquet metadata size limit to branch-4.1 (#67631) (#68047) ### What problem does this PR solve? Related PR: #67631 Backport #67631 to branch-4.1. FileScannerV2 incorrectly uses the Thrift RPC message ceiling to limit Parquet metadata. Add the independent, mutable `parquet_metadata_size_limit` with a 256 MiB default, while preserving the file-size check and enforcing the limit before metadata allocation and the second read. Resolved a test insertion conflict by retaining the existing physical-split refinement test and adding the upstream footer tests. The added and removed code matches the upstream patch. ### Release note FileScannerV2 uses the independent `parquet_metadata_size_limit` configuration, defaulting to 256 MiB. ### Check List (For Author) - Validation: clang-format 16 check and `git diff --check` passed; full diff self-reviewed. - Unit tests: preserved the upstream footer tests. A local `NewParquetReaderTest.NativeFooter*` run was attempted but stopped during a full rebuild; no local test result is claimed for this backport. CI validation is required. - Header hygiene: the check script is unavailable on the target branch. - Behavior changed: Yes; valid Parquet metadata above the RPC ceiling can be accepted within the dedicated metadata limit. - Does this need documentation: No. --- be/src/common/config.cpp | 4 ++++ be/src/common/config.h | 2 ++ be/src/format_v2/parquet/parquet_file_context.cpp | 14 +++++------- be/src/format_v2/parquet/parquet_file_context.h | 3 +-- be/test/format_v2/parquet/parquet_reader_test.cpp | 26 ++++++++++++++++++++--- 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/be/src/common/config.cpp b/be/src/common/config.cpp index 16d41bcc3e9..29ff784a2f2 100644 --- a/be/src/common/config.cpp +++ b/be/src/common/config.cpp @@ -1066,6 +1066,10 @@ DEFINE_mInt32(in_memory_file_size, "1048576"); // 1MB // Max size of parquet page header in bytes DEFINE_mInt32(parquet_header_max_size_mb, "1"); +// Max size of parquet file metadata in bytes +DEFINE_mInt64(parquet_metadata_size_limit, "268435456"); +DEFINE_Validator(parquet_metadata_size_limit, + [](const int64_t config) -> bool { return config > 0; }); // Max buffer size for parquet row group DEFINE_mInt32(parquet_rowgroup_max_buffer_mb, "128"); // Max buffer size for parquet chunk column diff --git a/be/src/common/config.h b/be/src/common/config.h index 65ea01ae452..7963d3331a5 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -1125,6 +1125,8 @@ DECLARE_mInt32(in_memory_file_size); // Max size of parquet page header in bytes DECLARE_mInt32(parquet_header_max_size_mb); +// Max size of parquet file metadata in bytes +DECLARE_mInt64(parquet_metadata_size_limit); // Max buffer size for parquet row group DECLARE_mInt32(parquet_rowgroup_max_buffer_mb); // Max buffer size for parquet chunk column diff --git a/be/src/format_v2/parquet/parquet_file_context.cpp b/be/src/format_v2/parquet/parquet_file_context.cpp index 02bf3400f66..8497e5ba529 100644 --- a/be/src/format_v2/parquet/parquet_file_context.cpp +++ b/be/src/format_v2/parquet/parquet_file_context.cpp @@ -120,13 +120,13 @@ Status NativeParquetMetadata::init_schema(bool enable_mapping_varbinary, namespace detail { -Status validate_native_footer_size(uint32_t serialized_size, size_t file_size, - size_t metadata_size_limit) { +Status validate_native_footer_size(uint32_t serialized_size, size_t file_size) { if (file_size < V2_PARQUET_FOOTER_SIZE || serialized_size > file_size - V2_PARQUET_FOOTER_SIZE) { return Status::Corruption("Parquet v2 footer size {} exceeds file size {}", serialized_size, file_size); } + const size_t metadata_size_limit = static_cast<size_t>(config::parquet_metadata_size_limit); if (serialized_size > metadata_size_limit) { return Status::Corruption("Parquet v2 footer size {} exceeds metadata limit {}", serialized_size, metadata_size_limit); @@ -235,13 +235,9 @@ Status parse_native_parquet_footer(io::FileReaderSPtr file, const uint32_t serialized_size = decode_fixed32_le(tail.data() + tail.size() - V2_PARQUET_FOOTER_SIZE); - // The configured Thrift message ceiling also bounds this file-controlled allocation. Keep the - // check before both allocation and the optional second read so a sparse file cannot force a - // process-sized metadata buffer merely by advertising a large footer. - const size_t metadata_size_limit = - static_cast<size_t>(std::max(config::thrift_max_message_size, 0)); - RETURN_IF_ERROR( - detail::validate_native_footer_size(serialized_size, file_size, metadata_size_limit)); + // Keep the dedicated metadata limit independent of RPC serialization limits, and enforce it + // before allocation so file-controlled footer sizes cannot create unbounded memory pressure. + RETURN_IF_ERROR(detail::validate_native_footer_size(serialized_size, file_size)); std::vector<uint8_t> serialized_metadata(serialized_size); if (serialized_size <= tail.size() - V2_PARQUET_FOOTER_SIZE) { const auto* metadata_start = diff --git a/be/src/format_v2/parquet/parquet_file_context.h b/be/src/format_v2/parquet/parquet_file_context.h index 9203cf0dccd..6f8de4f9f48 100644 --- a/be/src/format_v2/parquet/parquet_file_context.h +++ b/be/src/format_v2/parquet/parquet_file_context.h @@ -100,8 +100,7 @@ namespace detail { inline constexpr int64_t MAX_SERIALIZED_PARQUET_INDEX_BYTES = 64LL << 20; -Status validate_native_footer_size(uint32_t serialized_size, size_t file_size, - size_t metadata_size_limit); +Status validate_native_footer_size(uint32_t serialized_size, size_t file_size); std::string build_native_file_cache_key(std::string_view fs_name, std::string_view path, int64_t description_mtime, int64_t reader_mtime, diff --git a/be/test/format_v2/parquet/parquet_reader_test.cpp b/be/test/format_v2/parquet/parquet_reader_test.cpp index 3ec7493565d..756ea6236c9 100644 --- a/be/test/format_v2/parquet/parquet_reader_test.cpp +++ b/be/test/format_v2/parquet/parquet_reader_test.cpp @@ -3990,15 +3990,35 @@ TEST_F(NewParquetReaderTest, MutableUnknownVersionDeclinesPhysicalSplitRefinemen EXPECT_TRUE(children.empty()); } +TEST_F(NewParquetReaderTest, NativeFooterAcceptsMetadataAboveThriftMessageLimit) { + constexpr uint32_t metadata_size = 128UL << 20; + constexpr size_t file_size = 512UL << 20; + static_assert(metadata_size > 100UL << 20); + + EXPECT_EQ(config::parquet_metadata_size_limit, 256UL << 20); + EXPECT_TRUE( + format::parquet::detail::validate_native_footer_size(metadata_size, file_size).ok()); +} + TEST_F(NewParquetReaderTest, NativeFooterSizeIsBoundedBeforeMetadataAllocation) { - constexpr size_t file_size = 256UL << 20; - constexpr size_t metadata_limit = 100UL << 20; + constexpr size_t file_size = 512UL << 20; const auto status = format::parquet::detail::validate_native_footer_size( - static_cast<uint32_t>(metadata_limit + 1), file_size, metadata_limit); + static_cast<uint32_t>(config::parquet_metadata_size_limit + 1), file_size); EXPECT_TRUE(status.is<ErrorCode::CORRUPTION>()) << status; EXPECT_NE(status.to_string().find("metadata limit"), std::string::npos); } +TEST_F(NewParquetReaderTest, NativeFooterSizeCannotExceedFileSize) { + constexpr size_t file_size = 256UL << 20; + EXPECT_TRUE(format::parquet::detail::validate_native_footer_size( + static_cast<uint32_t>(file_size - 8), file_size) + .ok()); + const auto status = format::parquet::detail::validate_native_footer_size( + static_cast<uint32_t>(file_size - 7), file_size); + EXPECT_TRUE(status.is<ErrorCode::CORRUPTION>()) << status; + EXPECT_NE(status.to_string().find("file size"), std::string::npos); +} + TEST_F(NewParquetReaderTest, UnknownMtimeUsesPageCacheForImmutableFile) { _file_path = (_test_dir / "unknown_mtime_page_cache.parquet").string(); write_parquet_file(_file_path); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
