morrySnow commented on code in PR #64026:
URL: https://github.com/apache/doris/pull/64026#discussion_r3466233548
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionExprDateTrunc.java:
##########
@@ -135,10 +136,12 @@ public PartitionKeyDesc
generateRollUpPartitionKeyDesc(PartitionKeyDesc partitio
Preconditions.checkState(partitionKeyDesc.getLowerValues().size() == 1,
"only support one partition column");
DateTimeV2Literal beginTime = dateTrunc(
- partitionKeyDesc.getLowerValues().get(0).getStringValue(),
+
partitionKeyDesc.getLowerValues().get(0).getValue().getStringValue(),
Optional.empty(), false);
-
- PartitionValue lowerValue = new
PartitionValue(dateTimeToStr(beginTime, partitionColumnType));
+ String literalValue = PartitionExprUtil
+ .normalizePartitionValueString(dateTimeToStr(beginTime,
partitionColumnType), partitionColumnType);
+ PartitionValue lowerValue = new PartitionValue(
+ LiteralExprUtils.createLiteral(literalValue,
partitionColumnType));
Review Comment:
很多地方都有类似的代码。可不可以把这个过程放到 PartitionValue 中。比如 PartitionValue 提供一个构造函数，接受
string 和type？
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/MultiPartitionDesc.java:
##########
@@ -381,11 +403,83 @@ private static DateTimeFormatter dateFormat(TimeUnit
timeUnitType,
}
private DateTimeFormatter dateTypeFormat(String dateTimeStr) {
- String s = DATE_FORMAT;
- if (this.timeUnitType.equals(TimeUnit.HOUR) || dateTimeStr.length() ==
19) {
- s = DATETIME_FORMAT;
+ if (isTimestampTzFormat(dateTimeStr)) {
+ return dateTimeFormatter(stripTimeZone(dateTimeStr));
+ }
+ if (this.timeUnitType.equals(TimeUnit.HOUR) ||
isDateTimeFormat(dateTimeStr)) {
+ return dateTimeFormatter(dateTimeStr);
+ }
+ return DateTimeFormatter.ofPattern(DATE_FORMAT);
+ }
+
+ private String formatPartitionDateTime(LocalDateTime dateTime, String
originalDateTimeStr) {
Review Comment:
why need this step? add comment to explain why we need to formart the input
string
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonUtil.java:
##########
@@ -161,10 +163,10 @@ public static PaimonPartitionInfo
generatePartitionInfo(List<Column> partitionCo
Map<String, Partition> nameToPartition = Maps.newHashMap();
PaimonPartitionInfo partitionInfo = new
PaimonPartitionInfo(nameToPartitionItem, nameToPartition);
List<Type> types = partitionColumns.stream()
- .map(Column::getType)
+ .map(column -> column.getType())
.collect(Collectors.toList());
Map<String, Type> columnNameToType = partitionColumns.stream()
- .collect(Collectors.toMap(Column::getName, Column::getType));
+ .collect(Collectors.toMap(column -> column.getName(), column
-> column.getType()));
Review Comment:
这两行改动有点儿没必要
##########
fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java:
##########
@@ -301,6 +307,14 @@ public static ColumnRanges expressionToRanges(Expr expr,
}
LiteralExpr value = (LiteralExpr) slotBinding;
+ // Normalize the literal type to the partition column type so that
+ // compareLiteral works correctly for compatible type families
+ // (e.g. INT vs BIGINT, DATE vs DATETIME, STRING vs VARCHAR).
+ // A predicate such as CAST(k AS BIGINT) = 1 on an INT column
+ // produces a BIGINT-typed literal; without retyping,
+ // ColumnBound.compareTo() fails because the typed compareLiteral
+ // rejects cross-type comparisons.
+ value = normalizePartitionFilterLiteral(value,
desc.getColumn().getType());
Review Comment:
这里为什么会传入类型不一致的 literal？
##########
fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java:
##########
@@ -396,6 +506,8 @@ protected static PartitionColumnFilter
createPartitionFilter(SlotDescriptor desc
}
LiteralExpr literal = slotBinding instanceof PlaceHolderExpr
? ((PlaceHolderExpr) slotBinding).getLiteral() :
(LiteralExpr) slotBinding;
+ // Normalize to partition column type (see
normalizePartitionFilterLiteral).
+ literal = normalizePartitionFilterLiteral(literal,
desc.getColumn().getType());
Review Comment:
这里为什么会传入类型不一致的 literal？
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -655,27 +653,15 @@ public static Optional<Expression>
characterLiteralTypeCoercion(String value, Da
&& DateTimeChecker.isValidDateTime(value)) {
ret = DateTimeLiteral.parseDateTimeLiteral(value,
true).orElse(null);
} else if (dataType.isTimeStampTzType() &&
DateTimeChecker.isValidDateTime(value)) {
- if (DateTimeChecker.hasTimeZone(value)) {
- // Signature search can pass TIMESTAMPTZ(*) here.
TimestampTzLiteral rounds by scale,
- // so derive a concrete scale from the literal before
preserving its explicit offset.
- TimeStampTzType timeStampTzType = (TimeStampTzType)
dataType;
- if (timeStampTzType.getScale() < 0) {
- timeStampTzType =
TimeStampTzType.forTypeFromString(value);
- }
- ret = new TimestampTzLiteral(timeStampTzType, value);
- } else {
- DateTimeV2Literal dtV2Lit = (DateTimeV2Literal)
DateTimeLiteral
- .parseDateTimeLiteral(value,
true).orElse(null);
- if (dtV2Lit != null) {
- dtV2Lit = (DateTimeV2Literal)
(DateTimeExtractAndTransform.convertTz(
- dtV2Lit,
- new
StringLiteral(ConnectContext.get().getSessionVariable().timeZone),
- new StringLiteral("UTC")));
- ret = new TimestampTzLiteral(dtV2Lit.getYear(),
dtV2Lit.getMonth(), dtV2Lit.getDay(),
- dtV2Lit.getHour(), dtV2Lit.getMinute(),
dtV2Lit.getSecond(),
- dtV2Lit.getMicroSecond());
- }
+ // Signature search can pass TIMESTAMPTZ(*) here.
TimestampTzLiteral rounds by scale,
+ // so derive a concrete scale from the literal before parsing.
Review Comment:
按道理来讲，这里不应该能看到 TIMESTAMPTZ(*)
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -655,27 +653,15 @@ public static Optional<Expression>
characterLiteralTypeCoercion(String value, Da
&& DateTimeChecker.isValidDateTime(value)) {
ret = DateTimeLiteral.parseDateTimeLiteral(value,
true).orElse(null);
} else if (dataType.isTimeStampTzType() &&
DateTimeChecker.isValidDateTime(value)) {
- if (DateTimeChecker.hasTimeZone(value)) {
- // Signature search can pass TIMESTAMPTZ(*) here.
TimestampTzLiteral rounds by scale,
- // so derive a concrete scale from the literal before
preserving its explicit offset.
- TimeStampTzType timeStampTzType = (TimeStampTzType)
dataType;
- if (timeStampTzType.getScale() < 0) {
- timeStampTzType =
TimeStampTzType.forTypeFromString(value);
- }
- ret = new TimestampTzLiteral(timeStampTzType, value);
- } else {
- DateTimeV2Literal dtV2Lit = (DateTimeV2Literal)
DateTimeLiteral
- .parseDateTimeLiteral(value,
true).orElse(null);
- if (dtV2Lit != null) {
- dtV2Lit = (DateTimeV2Literal)
(DateTimeExtractAndTransform.convertTz(
- dtV2Lit,
- new
StringLiteral(ConnectContext.get().getSessionVariable().timeZone),
- new StringLiteral("UTC")));
- ret = new TimestampTzLiteral(dtV2Lit.getYear(),
dtV2Lit.getMonth(), dtV2Lit.getDay(),
- dtV2Lit.getHour(), dtV2Lit.getMinute(),
dtV2Lit.getSecond(),
- dtV2Lit.getMicroSecond());
- }
+ // Signature search can pass TIMESTAMPTZ(*) here.
TimestampTzLiteral rounds by scale,
+ // so derive a concrete scale from the literal before parsing.
+ TimeStampTzType timeStampTzType = (TimeStampTzType) dataType;
+ if (timeStampTzType.getScale() < 0) {
+ timeStampTzType = TimeStampTzType.forTypeFromString(value);
+ } else if (dataType.equals(TimeStampTzType.MAX) &&
!DateTimeChecker.hasTimeZone(value)) {
+ timeStampTzType = TimeStampTzType.forTypeFromString(value);
}
Review Comment:
这个分支是什么意思呢？为什么 MAX 也不 follow？
--
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]
