Jackie-Jiang commented on code in PR #19539:
URL: https://github.com/apache/pinot/pull/19539#discussion_r4113546451
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -105,6 +125,35 @@ public Set<String> prune(BrokerRequest brokerRequest,
Set<String> segments) {
return selectedSegments;
}
+ private Set<String> pruneWithPreparedPredicate(Expression filterExpression,
Set<String> segments) {
+ Set<String> selectedSegments = new HashSet<>();
+ List<PreparedPredicate> predicates = new ArrayList<>();
+ for (String segment : segments) {
+ SegmentPartitionInfo partitionInfo = _partitionInfoMap.get(segment);
+ if (partitionInfo == null || partitionInfo ==
SegmentPartitionUtils.INVALID_PARTITION_INFO) {
+ selectedSegments.add(segment);
+ continue;
+ }
+ PartitionFunction function = partitionInfo.getPartitionFunction();
+ int functionHashCode = function.hashCode();
+ PreparedPredicate predicate = null;
+ for (PreparedPredicate candidate : predicates) {
Review Comment:
Could we resolve the whole predicate to candidate partition IDs once per
distinct partition function? `EQUALS`/`IN` contribute IDs, `AND` intersects,
`OR` unions, and an unrelated predicate acts as a wildcard. A literal rejected
as invalid by the partition function can be omitted from its ID set; I’d catch
only expected invalid-input errors. This would leave one ID-overlap check per
segment instead of walking the predicate tree for every segment. Please
benchmark the common one-function case, including simple filters and wider
`AND`/`OR` filters, against the current lazy approach.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -105,6 +131,28 @@ public Set<String> prune(BrokerRequest brokerRequest,
Set<String> segments) {
return selectedSegments;
}
+ private Set<String> pruneWithPreparedPredicate(Expression filterExpression,
Set<String> segments) {
+ Set<String> selectedSegments = new HashSet<>();
+ Map<PartitionFunction, PreparedPredicate> predicates = new HashMap<>();
Review Comment:
Most queries appear to encounter only one or a few distinct partition
functions. Could we keep prepared results in a small list and promote to a
`HashMap` after several distinct functions, perhaps eight? That avoids hashing
a newly constructed function for every segment in the common case while
retaining fast lookup when functions vary widely. A lookup-only benchmark
favors the list for one function, though we should measure the full pruning
path before fixing the cutoff.
--
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]
