Skip to content

Commit 78f0bb6

Browse files
xiangfu0claude
andcommitted
Document reserved -1 sentinel contract on PartitionFunction.getPartition
Major fix: - PartitionFunction.getPartition(String) Javadoc now explicitly states the return-value contract: implementations must return non-negative partition ids in [0, getNumPartitions()), and -1 is reserved as a framework-internal sentinel for "expression evaluated to null" (PartitionPipelineFunction.NULL_RESULT_PARTITION_ID). Custom plugin partition functions must not return -1 as a real partition id; internal callers (broker pruner, stats collector, segment processing partitioner) treat -1 as "skip / no partition". Skipped findings (with rationale): - Broker pruner fail-open metric: requires introducing a new BrokerMeter; defer to a follow-up that adds the meter alongside similar pruning failure observability. - End-to-end integration test: substantial test infrastructure work (CustomDataQueryClusterIntegrationTest subclass with controller + realtime ingestion + broker routing assertions); defer. - PR-scope split: process feedback that requires coordinated landing with maintainers; out of scope for code-only fixes. - Cross-version controller-side gate: substantial Helix integration; documented hazard in ColumnPartitionConfig Javadoc, defer. - ColumnBoundPartitionFunction.evaluate null handling: reviewer's claim that legacy partition functions produced a deterministic partition id for null is incorrect — legacy PartitionFunction.getPartition(String) would NPE on value.getBytes(UTF_8). The wrapper's null→null conversion is a graceful improvement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6da3be7 commit 78f0bb6

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,15 @@ public interface PartitionFunction extends Serializable {
4747
* Method to compute and return partition id for the given value.
4848
* NOTE: The value is expected to be a string representation of the actual value.
4949
*
50+
* <p><b>Return-value contract:</b> implementations must return a non-negative partition id in
51+
* {@code [0, getNumPartitions())}. The value {@code -1} is reserved as a framework-internal sentinel for
52+
* "expression evaluated to null" (see
53+
* {@link org.apache.pinot.segment.spi.partition.pipeline.PartitionPipelineFunction#NULL_RESULT_PARTITION_ID}) —
54+
* custom plugin implementations must not return {@code -1} as a real partition id. Internal callers
55+
* (broker pruner, stats collector, segment processing partitioner) treat {@code -1} as "skip / no partition".
56+
*
5057
* @param value Value for which to determine the partition id.
51-
* @return partition id for the value.
58+
* @return partition id for the value (non-negative for real partitions; never {@code -1}).
5259
*/
5360
int getPartition(String value);
5461

0 commit comments

Comments
 (0)