Skip to content

fix(scan): share bucket pruning across append and key-value scans - #284

Open
wangyong9999 wants to merge 4 commits into
apache:mainfrom
wangyong9999:fix/append-scan-bucket-pruning
Open

fix(scan): share bucket pruning across append and key-value scans#284
wangyong9999 wants to merge 4 commits into
apache:mainfrom
wangyong9999:fix/append-scan-bucket-pruning

Conversation

@wangyong9999

@wangyong9999 wangyong9999 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: N/A.

Fixed-bucket append scans retain other buckets even when equality predicates fully specify the bucket key. The existing KV scan has the opposite problem after rescaling: it applies a single bucket computed from the current table option to files written with other bucket counts, which can drop matching data.

Both scans now use a shared selector in FileStoreScan. The converter builds the key row and bucket function once; each manifest entry is checked using its own TotalBuckets(). Explicit bucket filters keep their existing semantics and take precedence. Historical schemas remain eligible when their ordered bucket-key field IDs, Arrow types and bucket function match. Only incompatible bucket schemas or nonpositive total bucket counts fall back to the existing filtering behavior. Incomplete/non-equality predicates and bucket-unaware tables do not enable inference.

Inferred pruning runs at the manifest-entry level for both scan types. Manifest min/max-bucket skipping still requires an explicit filter. Inferred scans now also use the snapshot live-entry cache: its key includes the selected bucket, current bucket count and current schema ID, and its candidates retain files from other counts or schemas. Query predicates and the selector run after cache lookup. This restores repeated-lookup caching without losing historical files or leaking one query's filtering into another.

Bucket-schema compatibility is memoized per file schema ID. Historical schema reads use the existing ConcurrentHashMap utility because manifest filtering runs in parallel.

Decimal literals are converted to the field's precision and scale only when their value is preserved exactly. NaN literals disable inference because equal NaNs can have different stored hashes.

Tests

  • 56 focused core unit tests and 15 cache unit tests passed, covering per-entry bucket counts, compatible/incompatible historical bucket schemas, concurrent schema reads, and cache-key isolation. The two historical-schema pruning regressions failed on 9aca26db before this follow-up.
  • 81 integration tests passed across Parquet/ORC, prefetch and manifest-cache settings. Append and KV scans read historical files with current bucket options of 2, 4, 8 and 17. Fresh scans of the same snapshot report cache hits and return the correct rows.
  • Two append keys share a current two-bucket selection but need different historical buckets; both return correctly through the shared cache. Explicit-bucket entries are seeded first to check isolation from inferred candidates.
  • A negative control disabled only inferred cache eligibility, reproducing the old gate: all 8 cache-enabled Append/KV integration cases failed at the cache-enabled assertion. Restored the final source, rebuilt and reran the passing tests.
cmake --build build-reuse --target paimon-core-test paimon-scan-and-read-inte-test paimon-common-test -j12
build-reuse/release/paimon-core-test --gtest_filter='AppendBucketPruningTest.*:AppendOnlyFileStoreScanTest.*:BucketSelectConverterTest.*:KeyValueBucketPruningTest.*:KeyValueFileStoreScanTest.*:SchemaManagerTest.*:CastExecutorTest.TestDecimalToDecimal*'
build-reuse/release/paimon-common-test --gtest_filter='LruCacheTest.*'
build-reuse/release/paimon-scan-and-read-inte-test --gtest_filter='*Append*:*TestWithPKBucketSelectByPredicate*'
/home/wangyong.alen/.local/bin/uv run --no-project --python 3.11 --with pre-commit==3.8.0 pre-commit run --all-files
git diff --check

The incremental build passed with GCC 8, C++17 and default -Wall, without warnings. All pre-commit hooks passed. The full test suite, aarch64 and downstream KV RPC E2E were not run. Upstream CI requires maintainer approval; local checks are not a substitute for it.

Earlier measurement at 5ebbaf33, before the shared-selector follow-up (Release, local disk, warm OS cache, Parquet, prefetch off): 64 buckets, 4 manifests, 256 files, 16,384 rows with 512-byte seeded random values. Both versions used the same persisted table; each configuration warmed up for 40 lookups and measured 40. All lookups returned the expected four rows. The baseline restores only the append scan implementation from ead92078; the fixed version was measured both before and after baseline with similar results.

Mode Manifest cache Planned files Planning p50 ms Read p50 ms Scan + read p50 ms Scan + read p95 ms
Before append inference Off 256 3.972 73.118 77.087 87.784
Append inference at 5ebbaf33 Off 4 3.904 2.019 5.936 6.589
Before append inference On 256 3.884 74.083 78.008 79.683
Append inference at 5ebbaf33 On 4 3.849 2.010 5.853 5.953
Explicit bucket filter On 4 1.301 1.976 3.281 3.385

The improvement is in data-file reads. Planning time is essentially unchanged. This is a small local comparison with four manifests, not a many-manifest or downstream RPC benchmark.

API and Format

Added an overload of CacheKey::ForSnapshotLiveManifestEntries for inferred candidates scoped by bucket count and schema ID. The existing exported three-argument factory remains available. No protocol or storage format changes. Explicit bucket filtering retains its existing semantics.

Documentation

Updated docs/source/api/scan.rst with shared pruning, per-entry bucket counts, compatible historical schemas and inferred cache behavior.

Generative AI tooling

Generated-by: Codex (GPT-6)

Comment thread src/paimon/core/operation/append_only_file_store_scan.cpp Outdated
Comment thread src/paimon/core/operation/append_only_file_store_scan.cpp Outdated
Comment thread src/paimon/core/operation/append_only_file_store_scan.cpp Outdated
Comment thread src/paimon/core/operation/append_only_file_store_scan.cpp Outdated
@lxy-9602

lxy-9602 commented Sep 5, 2026

Copy link
Copy Markdown
Member

C++ already performs predicate-to-bucket conversion for KeyValueFileStoreScan, while this PR introduces a second append-specific path with different semantics. Java installs a shared total-aware selector for both scan types and computes the bucket using each manifest entry’s totalBucket. Could we centralize this in FileStoreScan and make the selector total-aware like java? Besides avoiding duplication, this would prevent the existing KV path from pruning historical files incorrectly after bucket rescaling. Thanks!

@wangyong9999 wangyong9999 changed the title feat(scan): prune append buckets from equality predicates fix(scan): share bucket pruning across append and key-value scans Sep 5, 2026
@wangyong9999

Copy link
Copy Markdown
Contributor Author

@lxy-9602 Fixed in 9aca26d. I reproduced the KV bug: with the current option set to 4 buckets, a matching file written with 2 buckets was discarded.

Selector setup and entry filtering now live in FileStoreScan. Both scans reuse the key row and bucket function, then compute the bucket from each entry's TotalBuckets(). The append-specific path and KV's inferred global bucket filter are removed.

Explicit bucket filters still take precedence. Different schema IDs retain the conservative fallback. Inferred KV scans now also skip the manifest-level bucket optimization and bucket-specific cache, since a single inferred bucket is unsafe across rescaling. The description calls out this tradeoff.

Added regressions for both scans and extended the real-file scan/read tests to current bucket options of 2, 4, 8 and 17. The two unit regressions failed before the fix; 46 unit tests and 81 integration tests pass now. The build and pre-commit checks pass too.

@wangyong9999

wangyong9999 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

The four workflows for the latest commit, 7c9545a4, are waiting for approval (action_required); no jobs have started. Could a maintainer approve them? Build and Test run.

Comment thread src/paimon/core/operation/file_store_scan.cpp Outdated
Comment thread src/paimon/core/operation/file_store_scan.cpp
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