Skip to content

Commit 9386980

Browse files
xiangfu0claude
andcommitted
Restore broker constructor compat shim; canonicalize normalizer; sunset
Critical fix: - SegmentPartitionMetadataManager: restore the legacy (String tableNameWithType, String partitionColumn, String partitionFunctionName, int numPartitions) constructor that existed on upstream master. Removing it broke binary compatibility for external broker plugins / vendor forks that linked against this signature. The shim builds a synthetic ColumnPartitionConfig and delegates to the new constructor; expression-mode tables must use the ColumnPartitionConfig overloads. Major fixes: - ColumnPartitionConfig: canonicalize partitionIdNormalizer to uppercase at construction time so downstream comparisons (mix of case-sensitive wire format and case-insensitive older comparisons) cannot disagree on case alone. - PartitionFunctionFactory + ColumnPartitionMetadata: add explicit "TODO: remove after release 1.7.0" sunset markers to all four remaining @deprecated entries on the partition surfaces, matching the existing sunset on SegmentPartitionConfig.getFunctionConfig. Skipped findings (with rationale): - Broker pruner string-form predicate (CRIT in prior reviews): plumbing typed values through requires substantial SPI refactor. - ByteArrayPartitionFunction normalizer mapping (claimed MASK by reviewer): on closer inspection abs(hashCode) % n is the ABS pattern (not MASK), so the existing return PartitionIntNormalizer .ABS.name() is correct. - Sentinel rename to "__functionExpr__": the literal "FunctionExpr" is now permanent on-the-wire and changing it would break test-built segments. The drift regression test already guards against future PartitionFunctionType enum collisions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5b13315 commit 9386980

4 files changed

Lines changed: 23 additions & 2 deletions

File tree

pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpartition/SegmentPartitionMetadataManager.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,19 @@ public class SegmentPartitionMetadataManager implements SegmentZkMetadataFetchLi
7575
private transient TablePartitionInfo _tablePartitionInfo;
7676
private transient TablePartitionReplicatedServersInfo _tablePartitionReplicatedServersInfo;
7777

78+
/**
79+
* Backward-compat shim: the legacy (name-mode-only) constructor used by external broker plugins / vendor forks
80+
* that linked against the pre-expression-mode signature. Builds a synthetic {@link ColumnPartitionConfig} and
81+
* delegates to the new constructor. Expression-mode tables must use the {@link ColumnPartitionConfig} overloads.
82+
* TODO: remove after release 1.7.0 once external callers have migrated.
83+
*/
84+
@Deprecated
85+
public SegmentPartitionMetadataManager(String tableNameWithType, String partitionColumn, String partitionFunctionName,
86+
int numPartitions) {
87+
this(tableNameWithType, partitionColumn,
88+
new ColumnPartitionConfig(partitionFunctionName, numPartitions), null);
89+
}
90+
7891
public SegmentPartitionMetadataManager(String tableNameWithType, String partitionColumn,
7992
ColumnPartitionConfig columnPartitionConfig) {
8093
this(tableNameWithType, partitionColumn, columnPartitionConfig, null);

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ public static PartitionFunction getPartitionFunction(String functionName, int nu
115115
* @deprecated Expression-mode configs require a column name to compile the pipeline. This overload throws on
116116
* expression-mode configs; prefer {@link #getPartitionFunction(String, ColumnPartitionConfig)} or
117117
* {@link #getPartitionFunction(String, ColumnPartitionConfig, FieldSpec)} which support both modes.
118+
* TODO: remove after release 1.7.0.
118119
* @throws IllegalArgumentException if {@code config.getFunctionExpr()} is non-null.
119120
*/
120121
@Deprecated
@@ -130,6 +131,7 @@ public static PartitionFunction getPartitionFunction(ColumnPartitionConfig confi
130131
*
131132
* @deprecated Expression-mode metadata requires a column name to compile the pipeline. This overload throws on
132133
* expression-mode metadata; prefer {@link #getPartitionFunction(String, ColumnPartitionMetadata)}.
134+
* TODO: remove after release 1.7.0.
133135
* @throws IllegalArgumentException if {@code metadata.getFunctionExpr()} is non-null.
134136
*/
135137
@Deprecated
@@ -160,7 +162,8 @@ public static PartitionFunction getPartitionFunction(String columnName, ColumnPa
160162
*
161163
* @deprecated For BYTES-typed partition columns this overload always compiles the expression pipeline with STRING
162164
* input, producing partition ids that disagree with ingestion. Prefer
163-
* {@link #getPartitionFunction(String, ColumnPartitionConfig, FieldSpec)}.
165+
* {@link #getPartitionFunction(String, ColumnPartitionConfig, int, FieldSpec)} (the FieldSpec-aware 4-arg form).
166+
* TODO: remove after release 1.7.0.
164167
*/
165168
@Deprecated
166169
public static PartitionFunction getPartitionFunction(String columnName, ColumnPartitionConfig columnPartitionConfig,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ public class ColumnPartitionMetadata {
7575
* @param functionConfig Configuration required by partition function.
7676
* @deprecated Use {@link #ColumnPartitionMetadata(PartitionFunction, Set)} instead, which derives all fields
7777
* directly from the {@link PartitionFunction} contract and keeps them consistent.
78+
* TODO: remove after release 1.7.0.
7879
*/
7980
@Deprecated
8081
public ColumnPartitionMetadata(String functionName, int numPartitions, Set<Integer> partitions,

pinot-spi/src/main/java/org/apache/pinot/spi/config/table/ColumnPartitionConfig.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.fasterxml.jackson.annotation.JsonCreator;
2222
import com.fasterxml.jackson.annotation.JsonProperty;
2323
import com.google.common.base.Preconditions;
24+
import java.util.Locale;
2425
import java.util.Map;
2526
import javax.annotation.Nullable;
2627
import org.apache.pinot.spi.config.BaseJsonConfig;
@@ -88,7 +89,10 @@ public ColumnPartitionConfig(@JsonProperty("functionName") @Nullable String func
8889
"Unsupported partitionIdNormalizer: %s", partitionIdNormalizer);
8990
_functionName = functionName;
9091
_functionExpr = functionExpr;
91-
_partitionIdNormalizer = partitionIdNormalizer;
92+
// Canonicalize to uppercase at construction so downstream comparisons (case-sensitive on the wire format,
93+
// case-insensitive in older code paths) cannot disagree on case alone.
94+
_partitionIdNormalizer = hasText(partitionIdNormalizer)
95+
? partitionIdNormalizer.trim().toUpperCase(Locale.ROOT) : null;
9296
_numPartitions = numPartitions;
9397
_functionConfig = functionConfig;
9498
}

0 commit comments

Comments
 (0)