github-actions[bot] commented on code in PR #64026:
URL: https://github.com/apache/doris/pull/64026#discussion_r3341299151
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/MultiPartitionDesc.java:
##########
@@ -269,8 +274,8 @@ private void trans() throws AnalysisException {
+ ", end column size is " +
partitionKeyDesc.getUpperValues().size() + ".");
}
- this.startString =
partitionKeyDesc.getLowerValues().get(0).getStringValue();
- this.endString =
partitionKeyDesc.getUpperValues().get(0).getStringValue();
+ this.startString =
partitionKeyDesc.getLowerValues().get(0).getValue().getStringValue();
+ this.endString =
partitionKeyDesc.getUpperValues().get(0).getValue().getStringValue();
Review Comment:
Using the typed literal's `getStringValue()` here breaks TIMESTAMPTZ
step/multi partitions. `LiteralExprUtils.createLiteral(..., TIMESTAMPTZ)`
stores a legacy `DateLiteral` whose string value is normalized like `2024-01-15
18:00:00.000000+00:00`; `trans()` then passes that to `dateFormat(...)`, but
`dateFormat` only accepts lengths 4/6/7/8/10/13/19 depending on the interval. A
valid Nereids statement such as `PARTITION BY RANGE(ts) FROM ('2024-01-15
13:00:00') TO ('2024-01-17 13:00:00') INTERVAL 1 DAY` on a `TIMESTAMPTZ(6)`
column is cast to TIMESTAMPTZ before building `MultiPartitionDesc`, then fails
with `Multi build partition start or end time style is illegal` instead of
generating partitions. Preserve the original partition text for format probing
(or teach the formatter/parser to accept the normalized TIMESTAMPTZ form) while
still using typed literals for the generated bounds.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java:
##########
@@ -63,8 +65,10 @@ public SinglePartitionDesc translateToCatalogStyle() {
partitionDataProperty, isInMemory, tabletType,
versionInfo, storagePolicy,
isMutable);
}
- List<PartitionValue> partitionValues =
-
values.stream().map(this::toLegacyPartitionValueStmt).collect(Collectors.toList());
+ List<PartitionValue> partitionValues = new java.util.ArrayList<>();
+ for (int i = 0; i < Math.min(values.size(), partitionTypes.size());
i++) {
+
partitionValues.add(toLegacyPartitionValueStmt(values.get(i).castTo(partitionTypes.get(i))));
+ }
Review Comment:
This `Math.min` changes validation semantics by silently dropping extra
boundary values before the catalog-side `PartitionKeyDesc.analyze(partColNum)`
check can reject them. For a single-column range table, `PARTITION p VALUES
LESS THAN (1, 2)` used to translate both values and then fail with `Partition
values number is more than partition column number`; now it translates only `1`
and can create a different partition than the user specified. Please keep all
values and validate cardinality explicitly before casting, rather than
truncating the list.
--
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]
