You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "baiyangtx (via GitHub)" <gi...@apache.org> on 2026/03/10 13:26:15 UTC
[PR] [core] Support push-down valueStats purne when read manifests. [paimon]
baiyangtx opened a new pull request, #7392:
URL: https://github.com/apache/paimon/pull/7392
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
Support push-down valueStats purne when read manifests.
This may reduce memory cost if manifest valueStats is huge
### Tests
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
### Generative AI tooling
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
Generated-by: Aime
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push-down valueStats prune when read manifests. [paimon]
Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx commented on PR #7392:
URL: https://github.com/apache/paimon/pull/7392#issuecomment-4646066854
@JingsongLi Good points. Updated:
- Removed the `loadFilter` parameter from `ObjectsCache.createSegments` — the cache now always stores full objects, so filtered subsets can never pollute the shared cache.
- Reverted the 6-param `read()` overload that was unwrapping filters. Added a minimal `read(key, fileSize, Filters, convertor)` overload instead, so `ManifestEntryFilters` passes through intact and `ManifestEntryCache` retains its indexed partition/bucket lookup.
PTAL when you have time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push-down valueStats prune when read manifests. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #7392:
URL: https://github.com/apache/paimon/pull/7392
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push-down valueStats prune when read manifests. [paimon]
Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx commented on PR #7392:
URL: https://github.com/apache/paimon/pull/7392#issuecomment-4589168889
@JingsongLi Thanks for the review — addressed all the comments. Could you take another look when you have time?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push-down valueStats prune when read manifests. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #7392:
URL: https://github.com/apache/paimon/pull/7392#issuecomment-4637978695
I reviewed this PR as part of a pass over recent open non-draft Paimon PRs. I did not find any clear correctness, compatibility, or regression issue to flag from the current diff.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push-down valueStats prune when read manifests. [paimon]
Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx commented on PR #7392:
URL: https://github.com/apache/paimon/pull/7392#issuecomment-4553338588
Thanks for the review, @JingsongLi
**Cache key:** The cache stores raw `InternalRow` bytes, not deserialized objects. Deserialization, filtering, and conversion run fresh on every cache hit. `dropStats` is applied as a post-processing loop after the cache read returns, so the same cached bytes are safe for both `dropStats=true` and `dropStats=false` queries.
**Correctness:** `filterByStats(entry)` executes inside `ManifestFile.read()` before the post-processing loop, so it always sees complete stats. And `dropStats` defaults to `false` — unaffected paths remain untouched.
Fixed the PR title typo as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push-down valueStats prune when read manifests. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #7392:
URL: https://github.com/apache/paimon/pull/7392#discussion_r3339441297
##########
paimon-core/src/main/java/org/apache/paimon/operation/AbstractFileStoreScan.java:
##########
@@ -493,20 +502,19 @@ private List<ManifestEntry> readManifest(
manifest.fileSize(),
manifestsReader.partitionFilter(),
createBucketFilter(),
- createEntryRowFilter().and(additionalFilter),
+ entryRowFilter.and(additionalFilter),
entry ->
(additionalTFilter == null || additionalTFilter.test(entry))
&& (manifestEntryFilter == null
|| manifestEntryFilter.test(entry))
&& filterByStats(entry));
- if (dropStats) {
- List<ManifestEntry> copied = new ArrayList<>(entries.size());
- for (ManifestEntry entry : entries) {
- copied.add(dropStats(entry));
- }
- entries = copied;
+
+ List<T> result = new ArrayList<>(entries.size());
+ for (ManifestEntry entry : entries) {
+ result.add(converter.apply(dropStats ? dropStats(entry) : entry));
Review Comment:
This still materializes the full `List<ManifestEntry>` from `ManifestFile.read()` before applying `dropStats`/`converter`, so peak memory during manifest read still includes all `valueStats`. That seems to miss the main goal of pushing the prune down into manifest reading: callers using `dropStats()` can still OOM before this loop gets a chance to copy the entries without stats.
##########
paimon-core/src/main/java/org/apache/paimon/utils/ObjectsCache.java:
##########
@@ -78,22 +98,38 @@ public List<V> read(K key, @Nullable Long fileSize, Filters<V> filters) throws I
fileSize = fileSizeFunction.apply(key);
}
if (fileSize <= cache.maxElementSize()) {
- segments = createSegments(key, fileSize);
+ segments = createSegments(key, fileSize, loadFilter);
Review Comment:
This makes the cache unsafe for any selective `loadFilter`: the cache key is still only `key`, so a cold read can store a filtered subset of the file, and a later read of the same key with a broader/different filter will hit that partial cache and silently miss rows. The new test currently asserts the first filtered read only; it should also read the same key again with `Filter.alwaysTrue()` and verify all rows are still returned. Could we avoid putting filtered segments into this cache, or include the load filter/projection in the cache identity?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org