github-actions[bot] commented on code in PR #64026:
URL: https://github.com/apache/doris/pull/64026#discussion_r3412455463
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java:
##########
@@ -564,7 +532,8 @@ public PartitionKey deserialize(JsonElement json,
java.lang.reflect.Type typeOfT
key = MaxLiteral.MAX_VALUE;
}
Review Comment:
This reset discards the full scalar type that was just deserialized from the
literal payload. That becomes unsafe with this PR because
`DecimalLiteral.compareLiteral` and `StringLiteral.compareLiteral` now require
exact `type.equals(...)`. For example, a persisted `DECIMAL(10,2)` partition
key is read back and overwritten to `Type.fromPrimitiveType(DECIMAL64)`
(`DECIMAL64(18,0)`), while a freshly built key for the same column uses
`DECIMAL64(10,2)`. Those keys then compare as different even though the value
is the same, which can break range lookup, overlap validation, and pruning
after replay/restart. The same issue applies to parameterized `VARCHAR`/`CHAR`
lengths. Please preserve the type from the deserialized literal when it is
present, falling back to `Type.fromPrimitiveType` only for old/missing
metadata, and add a serialization round-trip test that compares restored
DECIMAL and VARCHAR partition keys with freshly constructed keys for the column
type.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java:
##########
@@ -189,12 +200,74 @@ public void setPartitionTypes(List<DataType>
partitionTypes) {
this.partitionTypes = partitionTypes;
}
+ protected void ensurePartitionTypesInitialized() {
+ if (partitionTypes == null) {
+ throw new AnalysisException("partitionTypes should be initialized
before validating partition definition");
+ }
+ }
+
+ protected Expression typedPartitionExpression(Expression expression, int
index) {
+ ensurePartitionTypesInitialized();
+ return strictTypedPartitionExpression(expression,
partitionTypes.get(index));
Review Comment:
This strict conversion helper is not used by
`FixedRangePartition.validate()`, which still loops only to
`partitionTypes.size()`, drops any extra lower/upper values, and casts with
ordinary `castTo()`. That leaves a parallel RANGE syntax unfixed: on a
single-column table, `VALUES [(1, 2), (3, 4))` is rewritten to `[1, 3)` instead
of rejecting the extra values, and `DECIMAL(10,2)` fixed bounds such as
`"10.005"` still go through Nereids decimal casting with `HALF_UP` rounding
while `VALUES LESS THAN` now rejects them. Please route fixed-range bounds
through the same count validation and `strictTypedPartitionExpression` path,
and add fixed-range tests for extra values and decimal scale overflow.
--
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]
