JingsongLi commented on code in PR #9300:
URL: https://github.com/apache/paimon/pull/9300#discussion_r3817998434
##########
paimon-core/src/main/java/org/apache/paimon/table/system/SnapshotsTable.java:
##########
@@ -256,34 +256,43 @@ public void handleLeafPredicate(Predicate predicate,
String leafName) {
predicate.visit(LeafPredicateExtractor.INSTANCE).get(leafName);
if (snapshotPred != null) {
if (snapshotPred.function() instanceof Equal) {
- optionalFilterSnapshotIdMin =
- Optional.of((Long) snapshotPred.literals().get(0));
- optionalFilterSnapshotIdMax =
- Optional.of((Long) snapshotPred.literals().get(0));
+ long snapshotId = (Long) snapshotPred.literals().get(0);
+ updateMinSnapshotId(snapshotId);
+ updateMaxSnapshotId(snapshotId);
}
if (snapshotPred.function() instanceof GreaterThan) {
- optionalFilterSnapshotIdMin =
- Optional.of((Long) snapshotPred.literals().get(0)
+ 1);
+ updateMinSnapshotId((Long) snapshotPred.literals().get(0)
+ 1);
}
if (snapshotPred.function() instanceof GreaterOrEqual) {
- optionalFilterSnapshotIdMin =
- Optional.of((Long) snapshotPred.literals().get(0));
+ updateMinSnapshotId((Long) snapshotPred.literals().get(0));
}
if (snapshotPred.function() instanceof LessThan) {
- optionalFilterSnapshotIdMax =
- Optional.of((Long) snapshotPred.literals().get(0)
- 1);
+ updateMaxSnapshotId((Long) snapshotPred.literals().get(0)
- 1);
}
if (snapshotPred.function() instanceof LessOrEqual) {
- optionalFilterSnapshotIdMax =
- Optional.of((Long) snapshotPred.literals().get(0));
+ updateMaxSnapshotId((Long) snapshotPred.literals().get(0));
}
}
}
+ private void updateMinSnapshotId(long candidate) {
Review Comment:
[P1] Flatten the full AND before intersecting bounds
`PredicateBuilder.and` builds a balanced binary tree once there are 3+
predicates, but `withFilter` only iterates the two top-level children and
`handleLeafPredicate` reduces each nested subtree through
`LeafPredicateExtractor`, whose map retains only one predicate per field. For
example, `AND(snapshot_id >= 0, snapshot_id = 99, snapshot_id < 3)` drops the
nested equality and these helpers produce `[0, 2]`, so snapshots 1 and 2 are
returned even though the conjunction is empty. I reproduced this with a focused
test. The same traversal is used by `$schemas` and `$manifests`. Please flatten
the entire AND (for example with `PredicateBuilder.splitAnd`) and apply every
leaf, and add a 3-conjunct regression test.
##########
paimon-core/src/main/java/org/apache/paimon/table/system/SnapshotsTable.java:
##########
@@ -256,34 +256,43 @@ public void handleLeafPredicate(Predicate predicate,
String leafName) {
predicate.visit(LeafPredicateExtractor.INSTANCE).get(leafName);
if (snapshotPred != null) {
if (snapshotPred.function() instanceof Equal) {
- optionalFilterSnapshotIdMin =
- Optional.of((Long) snapshotPred.literals().get(0));
- optionalFilterSnapshotIdMax =
- Optional.of((Long) snapshotPred.literals().get(0));
+ long snapshotId = (Long) snapshotPred.literals().get(0);
+ updateMinSnapshotId(snapshotId);
+ updateMaxSnapshotId(snapshotId);
}
if (snapshotPred.function() instanceof GreaterThan) {
- optionalFilterSnapshotIdMin =
- Optional.of((Long) snapshotPred.literals().get(0)
+ 1);
+ updateMinSnapshotId((Long) snapshotPred.literals().get(0)
+ 1);
Review Comment:
[P2] Avoid overflow when converting exclusive bounds
For `snapshot_id > Long.MAX_VALUE`, the `+ 1` wraps to `Long.MIN_VALUE`, so
the range becomes effectively unbounded and all snapshots are returned; I
reproduced exactly that result locally. `snapshot_id < Long.MIN_VALUE` has the
symmetric `- 1` problem, and the same arithmetic appears in the
schemas/manifests readers. Please represent these impossible exclusive ranges
explicitly (or preserve exclusive bounds) instead of overflowing, with
regressions for both extrema.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
