This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new ea5efa367ca [fix](be) Backport Parquet metadata size limit to
branch-4.1 (#67631) (#68047)
ea5efa367ca is described below
commit ea5efa367ca4c5c520579a660b01d77c00aefdef
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]
