[fix](be) Disable Parquet binary min max pushdown#65494
Open
Gabriel39 wants to merge 1 commit into
Open
Conversation
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: FileScannerV2 returned Parquet footer min/max statistics as exact aggregate values. For BYTE_ARRAY and FIXED_LEN_BYTE_ARRAY columns, Parquet permits truncated lower and upper bounds, and Arrow 17 does not expose the exactness flags. MIN/MAX pushdown could therefore return a synthetic bound that does not occur in the data. Reject aggregate pushdown for these physical types so the upper reader falls back to a normal scan, while retaining exact numeric pushdown and safe statistics pruning.
### Release note
Fix incorrect Parquet MIN/MAX aggregate results for truncated binary statistics.
### Check List (For Author)
- Test: Unit Test
- ParquetScanTest targeted binary rejection and numeric success tests
- Behavior changed: Yes, Parquet binary MIN/MAX aggregate pushdown now falls back to scanning.
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review opinion: no blocking issue found. I did not find a substantiated correctness problem that needs an inline review comment.
Critical checkpoint conclusions:
- Goal and evidence: the PR disables Parquet MIN/MAX aggregate pushdown for BYTE_ARRAY and FIXED_LEN_BYTE_ARRAY physical types before footer statistics are converted into aggregate results. This matches the stated goal of avoiding inexact/truncated binary statistics as exact aggregate values.
- Scope and focus: the implementation is small and localized to Parquet aggregate pushdown, with focused BE unit coverage for binary rejection plus the existing numeric MIN/MAX success path.
- Fallback behavior:
Status::NotSupportedmaps toNOT_IMPLEMENTED_ERROR, andTableReader::_try_materialize_aggregate_pushdown_rowstreats that status as "do not push down" and continues with normal scanning, so rejecting binary stats should not become a query failure. - Parallel paths: COUNT pushdown is unchanged, numeric MIN/MAX remains enabled, and the existing ORC path is not affected by this Parquet-specific exactness gap.
- Compatibility/configuration/lifecycle/concurrency: no new config, protocol field, persistence format, transaction path, thread, or lifecycle boundary is introduced.
- Tests: the new unit test covers UTF8/BYTE_ARRAY and FIXED_LEN_BYTE_ARRAY rejection, and existing surrounding tests cover numeric MIN/MAX, row-group pruning, nested single-leaf projection, invalid requests, and missing statistics behavior.
- Test result correctness: the changed test asserts the exact fallback-triggering status for both binary physical types. I did not find an expected-output or session/config propagation issue.
- Observability/performance: the change trades only unsafe binary MIN/MAX pushdown for normal scan fallback; pruning still uses binary statistics through the existing statistics path.
Validation performed:
- Read the review prompt, code-review skill, required AGENTS.md files, existing review-thread context, changed-file list, user focus, and shared ledger.
- Reviewed the GitHub patch and surrounding Parquet aggregate pushdown, statistics conversion, TableReader fallback/materialization, and relevant BE unit tests.
- Ran scoped whitespace validation:
git diff --check HEAD -- be/src/format_v2/parquet/parquet_reader.cpp be/test/format_v2/parquet/parquet_scan_test.cpp. - Did not run BE unit tests in this checkout because
.worktree_initialized,thirdparty/installed, andthirdparty/installed/bin/protocare missing.
Subagent conclusions:
optimizer-rewrite: no candidates.tests-session-config: no candidates.- Convergence round 1: both live subagents reviewed the current ledger and empty proposed final comment set after the last ledger update, and both replied
NO_NEW_VALUABLE_FINDINGS.
User focus response:
- No additional user-provided focus was present in
.code-review.c6fFby/review_focus.txt.
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
FileScannerV2 returned Parquet footer min/max statistics as exact aggregate values. Parquet permits BYTE_ARRAY and FIXED_LEN_BYTE_ARRAY statistics to be truncated bounds, and the Arrow 17 statistics API does not expose the
is_min_value_exactandis_max_value_exactflags. A pushed-down MAX could therefore be a synthetic upper bound that does not occur in the data.This change rejects Parquet MIN/MAX aggregate pushdown for both binary physical types. The upper table reader receives
NotSupportedand falls back to a normal scan. Binary statistics remain available for safe pruning, COUNT is unchanged, and exact numeric MIN/MAX pushdown remains enabled.Release note
Fix incorrect Parquet MIN/MAX aggregate results for truncated binary statistics.
Check List (For Author)
ParquetScanTest.AggregateMinMaxRejectsInexactBinaryStatisticsParquetScanTest.AggregateCountAndMinMaxUseAllSelectedRowGroupsValidation on the designated Linux build host:
build-support/check-format.shrun-clang-tidy.shwas attempted; analysis reports pre-existing full-file complexity/style diagnostics andjni-util.htoolchain static assertions unrelated to this change.