xiangfu0 commented on code in PR #19539:
URL: https://github.com/apache/pinot/pull/19539#discussion_r4201927563
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java:
##########
@@ -58,6 +62,13 @@ default List<String> getNames() {
/// @return Number of possible partitions.
int getNumPartitions();
+ /// Whether query-local predicate preparation is useful for this function.
Functions whose instances cannot be
+ /// compared cheaply across segments can return false to use the original
per-segment pruning path.
+ default boolean supportsPartitionIdPreparation() {
+ return true;
Review Comment:
Done in e36cc19841. The SPI capability now defaults to false; the six
built-in functions with value equality explicitly opt in. BoundedColumnValue
and legacy identity-only plugins retain the original per-segment path. Unknown
subclasses also stay on that path unless they explicitly opt in.
Added a factory-loaded identity-only plugin regression test with 32 segments
and 128 IN literals: it performs 32 partition calculations, one first hit per
segment, without preparing or retaining per-segment candidate sets. The
existing equality/call-count tests still verify reuse for opted-in functions.
The new tests fail against 850bad9f22 and pass with this fix.
The companion StarTree update ea9777b2c1 explicitly opts Custom, Iceberg
Bucket, and Iceberg Truncate into preparation and keeps unknown subclasses
opted out. Validation passed 31 Apache partition-function cases, 42
broker-pruner cases, and 56 StarTree function cases plus the cross-repository
Custom pruning test. Module formatting, Checkstyle, and license checks pass.
Fresh CI is running.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -152,4 +237,107 @@ private boolean isPartitionMatch(Expression
filterExpression, SegmentPartitionIn
return true;
}
}
+
+ /// Computes conservative candidate IDs for the whole predicate. Null
represents every partition.
+ /// Partition columns are single-valued, so a row satisfying AND must belong
to every child's candidate set.
+ @Nullable
+ private IntSet preparePartitionIds(Expression expression, PartitionFunction
partitionFunction) {
+ Function function = expression.getFunctionCall();
+ FilterKind kind = FilterKind.valueOf(function.getOperator());
+ List<Expression> operands = function.getOperands();
+ switch (kind) {
+ case AND: {
+ IntSet ids = null;
+ for (Expression child : operands) {
+ IntSet childIds = preparePartitionIds(child, partitionFunction);
+ if (childIds != null) {
+ if (ids == null) {
+ ids = childIds;
+ } else {
+ // Singleton leaves are immutable; composite and IN sets can be
intersected in place.
+ if (!(ids instanceof IntOpenHashSet)) {
+ ids = new IntOpenHashSet(ids);
+ }
+ ids.retainAll(childIds);
+ }
+ }
+ }
+ return ids;
+ }
+ case OR: {
+ IntSet ids = new IntOpenHashSet();
+ boolean matchesAll = false;
+ for (Expression child : operands) {
+ IntSet childIds = preparePartitionIds(child, partitionFunction);
+ if (childIds == null) {
+ matchesAll = true;
Review Comment:
Done in e36cc19841. OR preparation now returns null immediately when a child
is unrestricted; the matchesAll flag is removed. Eager preparation still
combines reachable AND/OR branches, but an unrestricted OR child makes later
siblings unreachable, just as in the legacy evaluator.
The counting-function regression test uses an unrelated first child followed
by 128 IN literals (including an invalid final literal). All segments are
retained with zero partition calculations and no error. Against 850bad9f22 it
made 128 unnecessary calculations; with the fix it makes zero. All 42 focused
broker-pruner cases pass, including the existing short-circuit/error coverage.
--
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]
