xiangfu0 commented on code in PR #19539:
URL: https://github.com/apache/pinot/pull/19539#discussion_r4070957576
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -95,10 +98,12 @@ public Set<String> prune(BrokerRequest brokerRequest,
Set<String> segments) {
return segments;
}
Set<String> selectedSegments = new HashSet<>();
+ // A singleton has no repeated work to reuse. Keep its evaluation free of
predicate/cache setup.
+ QueryPartitionMatcher matcher = segments.size() > 1 ? new
QueryPartitionMatcher(filterExpression) : null;
Review Comment:
Revisited against current head 115e893c3f. Yes, preparing query-local state
can cost more than direct evaluation for small or non-reusable workloads. The
cutoff comparison at eight segments showed the configured fallback IN control
at 1.4352 -> 1.4927 us (+4.0%); at 32 it was 6.8524 -> 6.7020 us. This is why
the current PR retains the allocation-free legacy evaluator below the measured
default of 32 candidates, with live global/per-table overrides and
enablePartitionPruningCache=false as an escape hatch. Those measurements are
workload-specific, not a universal break-even claim.
For this PR I am retaining the measured two-path design. The benefit of a
single evaluator is real, but unconditionally preparing it would change the
small-query path; a shared nullable-cache evaluator would also need its own
performance comparison. The current boundary tests run the same predicate
fixtures at 31/32 candidates and with the cache forced off, including lazy
exceptions and unknown metadata. Zero candidates already return a fresh empty
set without evaluating the filter or preparing a predicate.
Resolving with this explicit performance/maintenance tradeoff, not claiming
the evaluators have been consolidated. Consolidation can be revisited
separately with equivalent small-query and fallback benchmarks. Current head
retains the previously verified 78 focused Apache and 46 StarTree passing
cases; no source changes were made during this re-review.
--
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]
