Skip to content

[fix](be) Disable Parquet binary min max pushdown#65494

Open
Gabriel39 wants to merge 1 commit into
apache:masterfrom
Gabriel39:codex/disable-parquet-string-minmax-pushdown
Open

[fix](be) Disable Parquet binary min max pushdown#65494
Gabriel39 wants to merge 1 commit into
apache:masterfrom
Gabriel39:codex/disable-parquet-string-minmax-pushdown

Conversation

@Gabriel39

Copy link
Copy Markdown
Contributor

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_exact and is_max_value_exact flags. 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 NotSupported and 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)

  • Test: Unit Test
    • ParquetScanTest.AggregateMinMaxRejectsInexactBinaryStatistics
    • ParquetScanTest.AggregateCountAndMinMaxUseAllSelectedRowGroups
  • Behavior changed: Yes. Parquet binary MIN/MAX aggregate pushdown now falls back to scanning.
  • Does this need documentation: No

Validation on the designated Linux build host:

  • build-support/check-format.sh
  • Targeted BE unit tests: 2 tests passed
  • run-clang-tidy.sh was attempted; analysis reports pre-existing full-file complexity/style diagnostics and jni-util.h toolchain static assertions unrelated to this change.

### 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
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39 Gabriel39 marked this pull request as ready for review July 12, 2026 05:38
@Gabriel39 Gabriel39 requested a review from yiguolei as a code owner July 12, 2026 05:38
@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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::NotSupported maps to NOT_IMPLEMENTED_ERROR, and TableReader::_try_materialize_aggregate_pushdown_rows treats 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, and thirdparty/installed/bin/protoc are 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.

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (13/13) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.74% (30173/40369)
Line Coverage 58.73% (331161/563891)
Region Coverage 55.48% (277781/500649)
Branch Coverage 56.73% (122747/216370)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants