[fix](be) Harden external reader filtering, statistics, and nested Parquet reads#65500
Merged
Gabriel39 merged 23 commits intoJul 13, 2026
Merged
Conversation
### 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
### What problem does this PR solve? Issue Number: N/A Related PR: apache#64456 Problem Summary: FileScannerV2 converts UTC timestamp statistics into session-local DATETIMEV2 values before ZoneMap evaluation. Across a daylight-saving time rollback, that conversion is not monotonic, so converted metadata endpoints may exclude local values that occur inside the UTC interval. Parquet row-group and page-index pruning, and ORC statistics pruning, could therefore skip matching data. Detect backward timezone transitions within UTC statistics ranges and conservatively disable min/max pruning for those ranges while retaining statistics for monotonic ranges and TIMESTAMPTZ mappings. ### Release note Fix incorrect Parquet and ORC file scan pruning for UTC timestamps across daylight-saving time rollbacks. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE UT: TimestampStatisticsTest.*, ParquetStatisticsTransformTest.*, and NewOrcReaderTest.AggregatePushdownTimestamp* (7 passed) - Behavior changed: Yes. Non-monotonic local timestamp statistics no longer prune rows or pages. - Does this need documentation: No
Disable unsafe ORC timestamp SARG pushdown in civil years with backward timezone transitions, while preserving Parquet min/max pruning for TIMESTAMPTZ values that retain UTC ordering. Add ORC, Parquet, and timezone transition regression coverage.
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Parquet row-group pruning cached Bloom filters only by leaf column id. After the first row group loaded a column Bloom filter, later row groups reused that filter and could be incorrectly pruned when a matching value existed only outside the first row group. Scope cache entries by both row group and leaf column so absence checks always use the current row group's Bloom filter. Add a parquet-mr fixture with two row groups and real Bloom data plus an enabled/disabled differential unit test.
### Release note
Fix incorrect Parquet Bloom filter pruning that could silently omit rows from later row groups.
### Check List (For Author)
- Test: Unit Test
- ParquetStatisticsPruningTest.BloomFilterCacheIsScopedToRowGroupAndColumn
- Behavior changed: Yes, Parquet Bloom pruning now uses each row group's own filter
- Does this need documentation: No
Issue Number: N/A
Related PR: N/A
Problem Summary: Parquet FLOAT and DOUBLE min/max statistics containing NaN were treated as valid by FileScannerV2. Doris comparisons can then interpret NaN as equal to predicate bounds, causing range and inequality predicates to incorrectly prune row groups or pages that contain matching finite values. Reject footer and ColumnIndex min/max statistics when either floating-point bound is NaN, while preserving null-count information and finite statistics.
Fix missing rows when Parquet floating-point min/max statistics contain NaN.
- Test: Unit Test
- ParquetStatisticsTransformTest targeted NaN footer and ColumnIndex tests
- Behavior changed: Yes, NaN floating-point min/max statistics are ignored for pruning.
- Does this need documentation: No
Issue Number: N/A Related PR: apache#65493 Problem Summary: A NaN min/max in one Parquet ColumnIndex page was reported as a transform failure, which disabled page-index pruning for the entire slot. Treat NaN bounds as unavailable only for that page so it is kept conservatively while later pages with finite bounds remain eligible for pruning. Preserve Parquet page-index pruning for finite pages when another page has NaN statistics. - Test: Unit Test - ParquetStatisticsTransformTest.* (4 tests passed) - Behavior changed: Yes. NaN page bounds no longer disable pruning for other pages in the slot. - Does this need documentation: No
### What problem does this PR solve? Issue Number: N/A Related PR: apache#65493 Problem Summary: When NaN invalidated Parquet min/max bounds, the statistics adapter returned no zonemap and discarded otherwise valid null-count metadata. Build a pass-all zonemap instead so range predicates remain conservative while IS NULL and IS NOT NULL can still prune from has_null and has_not_null for both footer and page-index statistics. ### Release note Preserve Parquet null-predicate pruning when floating-point min/max bounds contain NaN. ### Check List (For Author) - Test: Unit Test - ParquetStatisticsTransformTest.* (5 tests passed) - Behavior changed: Yes. Invalid range bounds no longer disable valid null-count pruning. - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
This was referenced Jul 12, 2026
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review completed for PR 65500 at head 6f4be2243b4fa9c3fe509b46daf961d7652fc932.
I did not find a substantiated correctness issue requiring an inline comment.
Critical checkpoint conclusions:
- Goal and tests: the PR hardens Parquet/ORC metadata pruning for missing null counts, DST rollback timestamp ranges, row-group bloom-filter cache scoping, and NaN min/max statistics. The changed code matches that goal in the reviewed paths, and targeted BE tests were added for each failure mode.
- Scope and focus: the changes are focused on BE format_v2 statistics/SARG pruning and matching unit tests.
- Data correctness: unsafe timestamp or NaN bounds fall back to no range pruning or aggregate pushdown failure; null-count-only pruning preserves only trustworthy
IS NULL/IS NOT NULLdecisions; bloom filters are now keyed by row group and leaf column. - Parallel paths: Parquet footer statistics, page-index statistics, aggregate min/max, ORC zone maps, ORC SARG construction, and TIMESTAMPTZ bypass paths were checked.
- Concurrency, lifecycle, config, persistence, and protocol compatibility: no new concurrency, persistent metadata, runtime config, FE/BE protocol, or storage-format compatibility path is introduced by this PR.
- Performance and observability: the timestamp rollback guard intentionally trades some metadata pruning in rollback years for correctness; no new observability requirement was identified.
- Test coverage: the added tests cover the changed behaviors statically. I did not run builds or tests because the review prompt forbids builds in this runner; the exact base commit is also not present locally, so
git diff --checkon the commit range could not run. I checked the authoritative bundled patch for added-line trailing whitespace instead. - User focus: no additional user-provided review focus was supplied.
### What problem does this PR solve? Issue Number: None Related PR: apache#65449 Problem Summary: FileScannerV2 split-level partition pruning classified predicates only by their referenced slots. A predicate could therefore reference only partition columns while also containing a zero-slot non-deterministic or error-preserving expression. Evaluating such a predicate once before reading the split could change its normal row-level semantics and incorrectly prune the whole file. Check the unwrapped predicate's existing safe-to-reorder contract before evaluating it at split level, and add unit coverage proving an unsafe runtime-filter predicate is neither executed nor used to prune a split. ### Release note Prevent FileScannerV2 partition pruning from pre-evaluating non-deterministic or error-preserving predicates. ### Check List (For Author) - Test: Unit Test - `TableReaderTest.PrepareSplit*` (2 tests passed) - `build-support/clang-format.sh` - `build-support/check-format.sh` - `git diff --check` - Behavior changed: Yes. Unsafe predicates remain on the normal row-level evaluation path instead of being evaluated once per split. - Does this need documentation: No
Issue Number: None Related PR: apache#65449 Problem Summary: Split pruning could skip an unsafe conjunct and still pre-execute later predicates. A later predicate could then prune a split before the earlier non-deterministic or error-preserving predicate reached its normal row-level evaluation point, changing conjunct ordering semantics. Preserve only the safe prefix for V2 partition pruning, V2 constant pruning, and the legacy FileScanner runtime-filter partition pruning path. Add unit coverage for all three paths. Split pruning now preserves row-level conjunct order when predicates are unsafe to pre-execute. - Test: Unit Test - ASAN BE unit tests: TableReaderTest.PrepareSplit*, TableReaderTest.ConstantPruningStopsAtUnsafePredicate, FileScannerTest.PartitionPruningStopsAtUnsafePredicate - C++ format check - Behavior changed: Yes. Split pruning stops at the first predicate that is unsafe to pre-execute. - Does this need documentation: No
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: FileScannerV2 localized predicate literals to the file column type whenever conversion succeeded, even when the conversion changed the literal value. For example, localizing a DOUBLE predicate value of 1.5 to an INT file column changed the boundary to 1 and could make file-level filtering silently drop matching rows. Require an exact round trip through the file type before localizing a literal; otherwise, preserve table semantics by casting the file slot. Exact conversions such as BIGINT 1 to INT 1 remain localized.
### Release note
Fix FileScannerV2 predicate localization to avoid missing rows after lossy literal conversion.
### Check List (For Author)
- Test: Unit Test
- ColumnMapperCastTest targeted literal localization tests
- Behavior changed: Yes, lossy literal conversions now fall back to casting the file slot.
- Does this need documentation: No
### What problem does this PR solve? Issue Number: N/A Related PR: apache#65490 Problem Summary: Round-trip equality alone can accept unsafe predicate localization when file values lose information during table materialization or when complex Field equality ignores nested contents. Restrict localization to scalar numeric file-to-table casts that preserve every file value, retain the exact literal round-trip check, and fall back to the table predicate for complex or unsupported casts. ### Release note Prevent file predicate localization from dropping rows for lossy schema-evolution casts. ### Check List (For Author) - Test: Unit Test - ColumnMapperCastTest (19 tests passed) - Behavior changed: Yes. Unsafe literal localization now falls back to evaluating the table-typed predicate after materialization. - Does this need documentation: No
### 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
Gabriel39
changed the title
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
I found two correctness issues that need changes before merge.
Checkpoint summary:
- Goal/test proof: the PR improves several pruning and metadata conservatism paths, but constant split pruning still has a safe-prefix gap and invalid timestamp metadata can still assert instead of falling back.
- Scope/focus: the changes are focused on BE file scanner and format_v2 metadata paths; I did not find unrelated refactoring issues.
- Parallel paths: legacy and V2 partition pruning now stop at unsafe predicates, but the constant-pruning path still iterates the reduced
_table_filterslist and misses slotless unsafe barriers. - Error handling/data quality: most missing, NaN, unsupported, and non-monotonic metadata cases fall back conservatively; inverted timestamp min/max is the remaining non-conservative case.
- Tests/validation: tests were not run because the review prompt forbids builds/tests in this runner.
.worktree_initializedandthirdparty/installedare missing, andgit diff --checkcould not run because the base object is absent in this shallow/grafted checkout. - User focus: no additional user-provided focus points were present.
- Convergence: all five subagents re-read the final two-comment set and returned
NO_NEW_VALUABLE_FINDINGS.
### What problem does this PR solve? Issue Number: None Related PR: apache#65500 Problem Summary: Corrupt external timestamp statistics with inverted bounds could terminate the BE, and a slotless unsafe conjunct could be omitted from localized table filters so a later constant predicate incorrectly pruned the split. Treat inverted statistics as unusable and track the constant-pruning safe prefix from the original conjunct order, including slotless ordering barriers. ### Release note Corrupt timestamp statistics and unsafe slotless predicates now fall back to conservative scan behavior. ### Check List (For Author) - Test: Regression test / Unit Test - TimestampStatisticsTest.* - TableReaderTest.ConstantPruningStopsAtUnsafePredicate - TableReaderTest.ConstantPruningStopsAtUnsafeSlotlessPredicate - Remote BE build - Behavior changed: Yes (split pruning now falls back conservatively for corrupt timestamp statistics and unsafe slotless predicates) - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-DS: Total hot run time: 179920 ms |
Contributor
ClickBench: Total hot run time: 25.05 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#65500 Problem Summary: Direct VARBINARY mappings localized late runtime filters into external file readers even though those readers intentionally do not support VARBINARY predicate pushdown. This could reject matching Iceberg BINARY partition rows. Keep VARBINARY filters for scanner-level evaluation after materialization. ### Release note Fix incorrect results for runtime filters on external VARBINARY columns, including Iceberg BINARY partition columns. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: ColumnMapperLocalizeFiltersTest.* (5 passed) - Behavior changed: Yes (VARBINARY runtime filters stay on scanner-level evaluation path) - Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: FileScannerV2 cloned its table-level conjuncts only when TableReader was
initialized. Runtime filters arriving after scanner open were visible to the scanner but never
reached subsequent split readers, so Parquet and ORC metadata pruning, native file filtering, and
JNI block filtering could not benefit from late runtime filters. Pass a freshly cloned conjunct
snapshot into every split, replace TableReader's initial snapshot during split preparation, and
prepare JNI conjuncts per split.
### Release note
Enable late runtime filters to participate in FileScannerV2 file-level filtering for subsequent
splits.
### Check List (For Author)
- Test: Unit Test
- `TableReaderTest.PrepareSplitReplacesInitialConjunctSnapshot`
- `FileScannerV2Test.RewriteSlotRefsToGlobalIndexMatrix`
- Behavior changed: Yes. Subsequent FileScannerV2 splits use the latest runtime-filter snapshot for
native and JNI filtering.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Complex Parquet column readers materialized every discarded row in a selection gap into one scratch column, so memory and decoding work grew with the entire gap. In addition, a refreshed split predicate could be bypassed by the table-level COUNT shortcut. Materialize skipped nested rows in bounded batches with per-batch scratch columns, and only use metadata row counts when the split has no active conjuncts.
### Release note
Bound memory usage while skipping nested Parquet columns and preserve late runtime-filter semantics for table-level COUNT.
### Check List (For Author)
- Test: Unit Test
- `ParquetColumnReaderControlTest.*` (16 tests)
- `TableReaderTest.PrepareSplitReplacesInitialConjunctSnapshot`
- `TableReaderTest.RefreshedConjunctDisablesTableLevelCount`
- Behavior changed: Yes. Nested Parquet skip work is processed in bounded batches, and table-level COUNT no longer bypasses active split predicates.
- Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65498 Problem Summary: A table-level metadata COUNT split can emit several synthetic batches across scheduler turns. If a runtime filter was still pending when the shortcut started, it could arrive after an unfiltered batch had already escaped, and the active split could not recover the corresponding real file rows. Pass the scanner runtime-filter completion state into split preparation and allow metadata COUNT only after every assigned runtime filter has arrived. ### Release note Table-level metadata COUNT now falls back to reading file rows while runtime filters are pending. ### Check List (For Author) - Test: Unit Test - TableReaderTest.PrepareSplitReplacesInitialConjunctSnapshot - TableReaderTest.RefreshedConjunctDisablesTableLevelCount - TableReaderTest.PendingRuntimeFilterDisablesTableLevelCount - Remote BE build - Behavior changed: Yes (metadata COUNT waits for all scanner runtime filters before using synthetic rows) - Does this need documentation: No
### What problem does this PR solve? Issue Number: close apache#65498 Related PR: apache#65498 Problem Summary: Nested Parquet skips rebuilt parent boundaries into scratch Columns and decoded leaf payloads that were immediately discarded. In addition, pending runtime filters disabled table-level metadata COUNT but could still allow normal COUNT or MIN/MAX aggregate pushdown to emit irreversible synthetic rows before the filter arrived. Add a levels-only nested consume protocol for scalar, list, map, and struct readers, preserving nullability and stream-alignment validation without payload materialization. Gate every aggregate pushdown path on the split runtime-filter snapshot. ### Release note Improve nested Parquet skip efficiency and preserve runtime-filter correctness for aggregate pushdown. ### Check List (For Author) - Test: Regression test / Unit Test - ParquetColumnReaderTest, ParquetColumnReaderBaseTest, ParquetColumnReaderControlTest, and targeted TableReader tests - Remote ./build.sh --be - Behavior changed: Yes (nested skip avoids decoding discarded payloads; aggregate pushdown waits for pending runtime filters) - Does this need documentation: No
### What problem does this PR solve? Issue Number: close apache#65498 Related PR: apache#65498 Problem Summary: The Parquet levels-only nested path called Arrow RecordReader::Reset() without first releasing ByteArray and FLBA builder chunks. Arrow requires GetBuilderChunks() to reset those builders, so a nested skip followed by a normal read mixed values from both batches and failed with values greater than the current definition/repetition levels. Release and immediately discard binary chunks after copying levels, without copying payloads into Doris Columns. Also derive the scalar consume slot threshold from schema metadata because levels-only batches intentionally omit value-slot metadata. ### Release note Fix nested Parquet queries that skip filtered rows before reading plain-encoded string values. ### Check List (For Author) - Test: Unit Test - 95 targeted Parquet reader, levels-only COUNT, and TableReader tests - Plain-encoded nested MAP skip-then-read regression test - Remote ./build.sh --be - Behavior changed: Yes (levels-only nested reads correctly reset Arrow binary builders) - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65500 Problem Summary: Combining the Parquet reader changes exposed that direct file mappings carry a slot-ref materialization projection. Treating every projection as unsafe disabled valid MIN/MAX statistics pushdown and made existing direct-mapping tests return raw rows. Use mapping triviality as the safety boundary: direct slot-ref mappings remain eligible while casts and other conversions still fall back to row scanning. ### Release note Fix MIN/MAX aggregate pushdown for direct external-file column mappings. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: ColumnMapperLocalizeFiltersTest.*, TableReaderTest.*, and ParquetColumnReaderTest.* (121 passed) - Behavior changed: Yes (direct mappings retain MIN/MAX statistics pushdown; converted mappings still fall back) - Does this need documentation: No
Gabriel39
changed the title
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review only: this checkout is missing .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc, so I did not run builds or tests.
I reviewed the prompt-specified scope, existing review threads/comments, changed-file list, authoritative diff, and the shared subagent ledger. After accepting these four findings and rerunning final convergence, all live final agents returned NO_NEW_VALUABLE_FINDINGS for the same final comment set.
Checkpoint conclusions: the PR is focused on external-reader predicate/statistics/nested-reader correctness, but the remaining issues below can still produce wrong scan or aggregate results. I did not find a separate concurrency, lifecycle, config, persistence, transaction, FE/BE protocol, or observability issue. The main parallel-path gaps are ORC non-timestamp stats and complex-root VARBINARY predicate localization. The added tests were reviewed statically only; they do not cover the four residual failures called out inline.
### What problem does this PR solve? Issue Number: None Related PR: apache#65500 Problem Summary: External reader review found four remaining correctness gaps: slotless conjuncts could still permit aggregate pushdown, TIMESTAMPTZ scale-mismatch filters could be localized with different rounding semantics, nested VARBINARY predicates could reach file readers, and decoded ORC statistics did not consistently reject inverted or NaN bounds. Disable aggregate pushdown whenever original conjuncts remain, keep unsafe filter conversions above the reader, and validate every decoded ORC statistics range before using it for metadata pruning or MIN/MAX aggregation. ### Release note Fix external table filtering and metadata aggregate correctness for slotless predicates, TIMESTAMPTZ scale mismatches, nested VARBINARY values, and invalid ORC statistics. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE unit tests: 20 targeted tests passed - Remote clang-format 16 check passed - Remote clang-tidy attempted; blocked by missing stddef.h and pre-existing diagnostics in the validation toolchain - Behavior changed: Yes, unsafe filter and aggregate pushdown paths now fall back to row-level evaluation - Does this need documentation: No
Gabriel39
changed the title
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
Static review found one additional non-duplicate correctness issue; I left one inline comment.
Checkpoint conclusions:
- Goal/tests: the PR hardens external-reader filtering, metadata fallback, and nested Parquet reading. Many previously reported paths are addressed or already covered by existing review threads, but the JNI conjunct lifecycle still has a standalone multi-split correctness gap. The added tests do not cover this JNI lifecycle case.
- Scope/focus: the changes are focused on BE scan/format_v2 code and matching tests. No additional user-provided review focus was supplied.
- Concurrency/thread safety: no new thread, lock-order, or shared-state concurrency issue was found in this pass.
- Lifecycle: the accepted issue is a lifecycle bug: preserved JNI VExprContext objects can be prepared again across splits while open() is skipped after the first split.
- Error handling: Status returns in the reviewed changed paths are checked; the accepted issue is not an ignored-Status problem.
- Memory safety/nullability: no separate memory-tracking, ownership, nullable, or ColumnConst/ColumnNullable issue was substantiated.
- Data correctness: the JNI lifecycle issue can make JNI block filtering fail or evaluate with stale/mismatched function state for predicates containing functions. Other mapper/statistics/nested-reader data-correctness concerns were either already commented in existing threads or conservatively handled in the current diff.
- Observability: no new observability requirement was identified for this patch.
- Config, compatibility, transaction/persistence, and FE/BE protocol: no new config item, persisted format, transaction, or protocol compatibility issue was found.
- Parallel paths: scanner-driven FileScannerV2 passes fresh split conjunct clones and is not the affected path; native TableReader and metadata fallback paths were rechecked against the existing comments and current diff.
- Tests/validation: static review only. I did not run builds or tests because the review prompt forbids builds in this runner; the exact base object is absent locally, and the checkout is missing worktree/dependency setup.
Convergence: three normal review agents plus two risk-focused agents re-read the final single-comment set and returned NO_NEW_VALUABLE_FINDINGS.
Contributor
TPC-H: Total hot run time: 29308 ms |
Contributor
TPC-DS: Total hot run time: 180613 ms |
Contributor
ClickBench: Total hot run time: 25.11 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Jul 13, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
hello-stephen
pushed a commit
that referenced
this pull request
Jul 13, 2026
### What problem does this PR solve? Fix a BE UT compile failure introduced by the combination of #65500 and #65503. `SplitReadOptions` now has `conjuncts` and `all_runtime_filters_applied`, while the new `jni_table_reader_test.cpp` designated initializer did not initialize them. With `-Werror,-Wmissing-designated-field-initializers`, this fails compilation. This PR explicitly initializes the two fields in the test and adds the missing `<optional>` include for `std::nullopt`.
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
3 participants
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: None
Related PR: Supersedes #65471, #65489, #65490, #65493, #65494, #65497, #65498, and #65499.
Problem Summary: FileScannerV2 and its Parquet/ORC readers had several correctness and resource-control gaps:
This PR consolidates the fixes into one external-reader hardening change. It refreshes predicates for every split, waits for runtime-filter completion before aggregate shortcuts, disables aggregate pushdown whenever original conjuncts remain, preserves only safe conjunct prefixes, and keeps VARBINARY and TIMESTAMPTZ scale-mismatch filters above the file reader. It also validates all decoded ORC statistics bounds, makes invalid or inexact Parquet/ORC statistics fall back conservatively, scopes Bloom filters by row group and column, permits MIN/MAX only for order-preserving trivial mappings, and processes nested Parquet skips in bounded levels-only batches with correct binary-builder reset.
Release note
Fix incorrect external-file filtering, partition/statistics pruning, and COUNT/MIN/MAX pushdown. Unsafe predicates or statistics now fall back to row-level scanning, late runtime filters use current split state, and nested Parquet skips use bounded memory without decoding discarded payloads.
Check List (For Author)
gabriel@10.26.20.3: combined ColumnMapper, TableReader, and Parquet reader suite (121 tests passed).build-support/check-format.shwith clang-format 16 passed across 4,237 C/C++ files.stddef.hand pre-existing diagnostics outside these changes.