github-actions[bot] commented on code in PR #64026:
URL: https://github.com/apache/doris/pull/64026#discussion_r3419553162
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -298,25 +300,45 @@ private ArrayList<AddPartitionOp>
getAddPartitionOp(Database db, OlapTable olapT
nowPartitionPrevBorder,
dynamicPartitionProperty.getTimeUnit());
for (; idx <= dynamicPartitionProperty.getEnd(); idx++) {
- String prevBorder = DynamicPartitionUtil.getPartitionRangeString(
- dynamicPartitionProperty, now, idx, partitionFormat);
- String nextBorder = DynamicPartitionUtil.getPartitionRangeString(
- dynamicPartitionProperty, now, idx + 1, partitionFormat);
- PartitionValue lowerValue = new PartitionValue(prevBorder);
- PartitionValue upperValue = new PartitionValue(nextBorder);
-
boolean isPartitionExists = false;
Range<PartitionKey> addPartitionKeyRange;
+ PartitionValue lowerValue;
+ PartitionValue upperValue;
+ String prevBorder;
+ String nextBorder;
+ try {
+ prevBorder = DynamicPartitionUtil.getPartitionRangeString(
+ dynamicPartitionProperty, now, idx, partitionFormat);
+ prevBorder =
PartitionExprUtil.normalizePartitionValueString(prevBorder,
partitionColumn.getType());
+ nextBorder = DynamicPartitionUtil.getPartitionRangeString(
+ dynamicPartitionProperty, now, idx + 1,
partitionFormat);
+ nextBorder =
PartitionExprUtil.normalizePartitionValueString(nextBorder,
partitionColumn.getType());
+ lowerValue = new PartitionValue(
+ LiteralExprUtils.createLiteral(prevBorder,
partitionColumn.getType()));
+ upperValue = new PartitionValue(
+ LiteralExprUtils.createLiteral(nextBorder,
partitionColumn.getType()));
+ } catch (AnalysisException e) {
+ // Normalization or literal creation error: should not happen
for
+ // valid dynamic partition configurations.
+ LOG.warn("Error normalizing partition values. db: {}, table:
{}, partition idx: {}",
+ db.getFullName(), olapTable.getName(), idx, e);
+ if (executeFirstTime) {
+ throw new DdlException("error normalizing partition
values, error: "
+ + e.getMessage());
+ }
+ continue;
+ }
+
try {
PartitionKey lowerBound =
PartitionKey.createPartitionKey(Collections.singletonList(lowerValue),
Collections.singletonList(partitionColumn));
PartitionKey upperBound =
PartitionKey.createPartitionKey(Collections.singletonList(upperValue),
Collections.singletonList(partitionColumn));
addPartitionKeyRange = Range.closedOpen(lowerBound,
upperBound);
- } catch (Exception e) {
- // AnalysisException: keys.size is always equal to
column.size, cannot reach this exception
- // IllegalArgumentException: lb is greater than ub
- LOG.warn("Error in gen addPartitionKeyRange. db: {}, table:
{}, partition idx: {}",
Review Comment:
This catch no longer covers the failure described by the comment.
`Range.closedOpen(lowerBound, upperBound)` throws `IllegalArgumentException`
when `lowerBound > upperBound`; it does not throw `AnalysisException`. Before
this PR, the `catch (Exception)` path skipped the bad index or converted it to
a `DdlException` on first execution. With the narrowed catch, a too-small
`dynamic_partition.start` / reversed generated range can escape the scheduler
as an unchecked exception instead of preserving the previous failure handling.
Please catch `IllegalArgumentException` here as well, or compare the two
`PartitionKey`s before calling `Range.closedOpen`.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java:
##########
@@ -367,6 +383,92 @@ public static ColumnRanges expressionToRanges(Expr expr,
}
}
+ /**
+ * Re-type a literal value to the partition column's type when the two
+ * primitive types belong to the same family (fixed-point, date, string,
etc.)
+ * but differ in detail (e.g. INT vs BIGINT, DATE vs DATETIMEV2, STRING vs
+ * VARCHAR). This is necessary because {@link ColumnBound} delegates to
+ * {@link PartitionKey#compareLiteralExpr} which rejects cross-type
+ * comparisons. For example, a predicate {@code CAST(k AS BIGINT) = 1} on
an
+ * INT partition column produces a BIGINT-typed literal that would fail to
+ * compare against INT partition bounds.
+ *
+ * <p>The re-parse is a best-effort: if {@code value} already has the exact
+ * column type, or the types are in different families, it is returned
+ * unchanged.
+ */
+ @VisibleForTesting
+ static LiteralExpr normalizePartitionFilterLiteral(LiteralExpr value, Type
columnType) {
+ Type valueType = value.getType();
+ if (valueType.equals(columnType)) {
+ return value;
+ }
+ if (!valueType.isFixedPointType() && !valueType.isDateType()
+ && !valueType.isStringType() && !valueType.isDecimalV3()) {
+ return value;
+ }
+ // Only re-type within the same family.
+ if (!isSamePrimitiveFamily(valueType, columnType)) {
+ return value;
+ }
+ try {
+ return LiteralExprUtils.createLiteral(value.getStringValue(),
columnType);
+ } catch (AnalysisException e) {
+ // String-based re-parsing failed (e.g. DATETIME literal for a
+ // DATE partition column). For date-like types, construct a
+ // target-typed literal directly from the source literal's fields.
Review Comment:
This lossy DATETIME/TIMESTAMP-to-DATE conversion is not conservative for
range predicates because this helper does not know the operator. For example,
on a `DATEV2` partition column `k` with partition `[2024-01-01, 2024-01-02)`,
the predicate `CAST(k AS DATETIMEV2) < DATETIMEV2 '2024-01-01 12:00:00'` can
match rows in that partition because the DATE value casts to midnight. This
code normalizes the bound to `DATEV2 '2024-01-01'`, after which
`expressionToRanges()` builds `Range.lessThan('2024-01-01')` and
`createPartitionFilter()` uses an exclusive upper bound at the same date, so
the partition starting at `2024-01-01` can be pruned incorrectly. Please avoid
lossy date-time to date normalization in this operator-agnostic helper, or make
the normalization operator-aware and widen/skip pruning when the time part is
non-midnight.
--
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]
