xiangfu0 commented on code in PR #19539:
URL: https://github.com/apache/pinot/pull/19539#discussion_r4190949930
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -152,4 +223,106 @@ private boolean isPartitionMatch(Expression
filterExpression, SegmentPartitionIn
return true;
}
}
+
+ /// Lazily prepares only visited expressions and values. Instances belong to
one prune call, never shared by queries.
+ private final class PreparedPredicate {
+ private final Expression _expression;
+ private final PartitionFunction _partitionFunction;
+ private FilterKind _kind;
+ private List<Expression> _operands;
+ private PreparedPredicate[] _children;
+ private boolean _isPartitionPredicate;
+ private Integer _singlePartitionId;
+ private IntSet _partitionIds;
+ private int _numEvaluatedValues;
+
+ private PreparedPredicate(Expression expression, PartitionFunction
partitionFunction) {
+ _expression = expression;
+ _partitionFunction = partitionFunction;
+ }
+
+ private boolean matches(Set<Integer> partitions) {
Review Comment:
Agreed that the nested AND/OR case needed more work. I rebased and measured
256/512/1024 candidates with the same two-fork full-prune JMH fixture. The
previous lazy path was 6.14/13.60/28.77 µs; the updated PR is 3.34/7.27/15.02
µs. It caches a direct OR's partition-ID union only after a segment misses
every branch, so earlier matches still skip unvisited values and later segments
avoid traversing the OR tree.
I did not adopt the temporary whole-predicate eager implementation:
flattening AND into an ID intersection is incorrect when a segment advertises
multiple partitions (e.g. {1,2} should satisfy `id=1 AND id=2` under the
current conservative pruner semantics), and eager parsing can throw for
literals an earlier branch would have skipped. That temporary version also
measured 4.97 vs 3.73 µs for first-hit IN(128) at 256 segments and 10.16 vs
8.31 at 512. The final code keeps those paths lazy; isolated 256-segment
first-hit recheck was 3.58 vs 3.62 µs, though some runs were noisy, so I'm not
claiming a precise small-case win. The PR description now includes the method
and limits. All 30 focused broker cases pass, including a multi-partition
segment after OR preparation; exact-head CI is running.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
