Skip to content

[fix](be) Preserve row groups when null count is missing#65499

Closed
Gabriel39 wants to merge 1 commit into
apache:masterfrom
Gabriel39:codex/fix-parquet-missing-null-count
Closed

[fix](be) Preserve row groups when null count is missing#65499
Gabriel39 wants to merge 1 commit into
apache:masterfrom
Gabriel39:codex/fix-parquet-missing-null-count

Conversation

@Gabriel39

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: FileScannerV2 treated a missing optional Parquet null_count as proof that a row group contained no NULL values. When min/max statistics were present, IS NULL predicates could therefore prune a row group that actually contained NULL rows. This change treats unknown null counts conservatively as possibly containing NULL while retaining available min/max statistics, and covers the metadata combination with a unit test.

Release note

Fix incorrect IS NULL results for Parquet files whose writers omit null_count statistics.

Check List (For Author)

  • Test: Unit Test
    • ParquetStatistics* BE unit tests (7 tests, 0 failures)
    • build-support/check-format.sh
    • build-support/run-clang-tidy.sh --build-dir be/ut_build_ASAN
  • Behavior changed: Yes, row groups with unknown Parquet null counts are no longer discarded by IS NULL pruning
  • Does this need documentation: No

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: FileScannerV2 treated a missing optional Parquet null_count as proof that a row group contained no NULL values. When min/max statistics were present, IS NULL predicates could therefore prune a row group that actually contained NULL rows. Treat unknown null counts conservatively as possibly containing NULL while retaining available min/max statistics, and cover the metadata combination with a unit test.

### Release note

Fix incorrect IS NULL results for Parquet files whose writers omit null_count statistics.

### Check List (For Author)

- Test: Unit Test
    - ParquetStatistics* BE unit tests
- Behavior changed: Yes, row groups with unknown Parquet null counts are no longer discarded by IS NULL pruning
- 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 Gabriel39 marked this pull request as ready for review July 12, 2026 07:19
@Gabriel39 Gabriel39 requested a review from yiguolei as a code owner July 12, 2026 07:19
@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@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.

Automated review complete. I found no blocking correctness issue and no inline comments to submit.

Critical checkpoint conclusions:

  • Goal and tests: The PR fixes FileScannerV2 Parquet row-group pruning when optional null_count is absent by treating null presence as unknown/possible instead of false. The added BE unit test covers the key metadata shape: min/max retained while has_null_count is false.
  • Scope and focus: The change is narrow and focused: one statistics conversion line plus a targeted unit test.
  • Correctness: The row-group zone-map path is conservative for IS NULL after the change. Row-level VExpr predicates still execute after metadata pruning. Page-index pruning remains separately conservative when page null counts are unavailable.
  • Parallel paths: The legacy Parquet reader disables its zone map when row-group null_count is missing, so it does not repeat the false-prune behavior. Min/max aggregate pushdown only consumes has_min_max, so the changed null flag does not alter aggregate min/max computation.
  • Concurrency, lifecycle, config, compatibility, persistence, and write paths: Not involved by this patch.
  • Tests and style: clang-format-16 --dry-run --Werror passed on the two changed files. I did not run the BE unit test because this review checkout lacks thirdparty/installed and thirdparty/installed/bin/protoc.

Subagent conclusions:

  • optimizer-rewrite: no optimizer/rewrite, semantic-equivalence, parallel join, or aggregate candidates.
  • tests-session-config: no tests/session/config, compatibility, expected-output, or CI/style candidates.
  • Final convergence round 1 ended with both live subagents returning NO_NEW_VALUABLE_FINDINGS for the same empty proposed inline comment set.

User focus: No additional user-provided review focus was present.

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.77% (30182/40368)
Line Coverage 58.72% (331082/563878)
Region Coverage 55.53% (278003/500634)
Branch Coverage 56.77% (122824/216359)

@Gabriel39

Copy link
Copy Markdown
Contributor Author

Superseded by #65500, which consolidates and validates the four related fixes.

@Gabriel39 Gabriel39 closed this Jul 12, 2026
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