xiangfu0 commented on code in PR #19539:
URL: https://github.com/apache/pinot/pull/19539#discussion_r4110437293
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -994,6 +1000,10 @@ public static class QueryOptionKey {
/// Query-level override for `inpredicate.threshold`. Negative means
always prune.
public static final String IN_PREDICATE_PRUNING_THRESHOLD =
"inPredicatePruningThreshold";
+ /// Set false to use the uncached single-column broker partition
pruner. Defaults to true; the cache is only
+ /// used at the configured minimum segment count (default 32). Does
not disable partition pruning itself.
+ public static final String ENABLE_PARTITION_PRUNING_CACHE =
"enablePartitionPruningCache";
Review Comment:
Addressed in 0ca0c8641a. The query option is now numeric:
`partitionPruningMinSegments`. It overrides the table or cluster threshold, and
a negative value disables preparation while keeping partition pruning enabled.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java:
##########
@@ -58,11 +58,20 @@ default List<String> getNames() {
/// @return Number of possible partitions.
int getNumPartitions();
+ /// Returns the configuration used to construct this function, or null if
none is exposed.
@Nullable
default Map<String, String> getFunctionConfig() {
return null;
}
+ /// Returns whether this function and `other` produce identical partition
ids for every input.
+ /// Implementations may opt in using immutable value equality over every
output-affecting setting.
+ /// Equality must be reflexive, symmetric and transitive; matching hash
codes alone are insufficient.
+ /// The default permits reuse only for the same instance, including for
existing plugins with unrelated equality.
+ default boolean canReusePartitionIds(PartitionFunction other) {
+ return this == other;
Review Comment:
Done in 0ca0c8641a. The default now delegates to `equals(other)`, and the
built-ins no longer repeat that override.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -105,6 +125,30 @@ 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<>();
+ PreparedPredicate predicate = null;
+ PartitionFunction cachedFunction = null;
Review Comment:
Updated in 0ca0c8641a. The query-local path now keeps a small list with one
prepared predicate per distinct compatible partition function. The hash is only
a fast precheck; `canReusePartitionIds()` confirms equality before reuse.
##########
pinot-common/src/main/java/org/apache/pinot/common/partition/function/ByteArrayPartitionFunction.java:
##########
@@ -64,6 +64,32 @@ public PartitionIdNormalizer getPartitionIdNormalizer() {
return _normalizer;
}
+ @Override
+ public boolean canReusePartitionIds(PartitionFunction other) {
+ return equals(other);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (this == other) {
+ return true;
+ }
+ // Subclasses may introduce partitioning state; they must explicitly
define their own equality.
+ if (other == null || getClass() != ByteArrayPartitionFunction.class
Review Comment:
Updated in 0ca0c8641a. All six built-ins now use the standard identity,
null, and runtime-class checks in `equals()`.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -339,6 +339,12 @@ public static class Instance {
}
public static class Broker {
+ /// Minimum candidate segments for query-local partition-ID caching.
Cluster config supports per-table overrides
+ /// by appending `.<tableNameWithType>` to this key. Zero enables caching
for any candidate count.
Review Comment:
Addressed in 0ca0c8641a. The per-table override now lives in
`RoutingConfig.partitionPruningMinSegments`; the cluster config supplies only
the default. A negative value disables preparation.
--
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]
