morrySnow commented on code in PR #64026:
URL: https://github.com/apache/doris/pull/64026#discussion_r3397527696
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java:
##########
@@ -189,12 +200,73 @@ 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));
+ }
+
+ static Expression strictTypedPartitionExpression(Expression expression,
DataType targetType) {
+ if (expression instanceof MaxValue || expression instanceof
MaxLiteral) {
+ return expression;
+ }
+ if (expression.isNullLiteral()) {
+ return expression.checkedCastTo(targetType);
+ }
+ if (!expression.isLiteral()) {
+ throw new AnalysisException("Partition value must be literal: " +
expression.toSql());
+ }
+ String value = ((Literal) expression).getStringValue();
+ try {
+ if (targetType.isDateTimeType() || targetType.isDateTimeV2Type()
|| targetType.isTimeStampTzType()) {
+ return convertPartitionLiteral(value,
targetType).checkedCastTo(targetType);
+ }
+ validateCharacterLength(value, targetType);
+ LiteralExpr typedLiteral = LiteralExprUtils.createLiteral(
+ value, targetType.toCatalogDataType());
+ return Literal.fromLegacyLiteral(typedLiteral,
typedLiteral.getType());
+ } catch (org.apache.doris.common.AnalysisException e) {
+ throw new AnalysisException(e.getMessage(), e);
+ }
+ }
+
+ private static void validateCharacterLength(String value, DataType
targetType) {
+ if (!targetType.isCharType() && !targetType.isVarcharType()) {
+ return;
+ }
+ CharacterType characterType = (CharacterType) targetType;
+ if (characterType.isLengthSet() && value.length() >
characterType.getLen()) {
+ throw new AnalysisException(String.format(
+ "Partition value %s's length exceeds type length: %d > %d
for %s",
+ value, value.length(), characterType.getLen(),
targetType));
+ }
+ }
+
+ private static Literal convertPartitionLiteral(String value, DataType
targetType) {
+ if (targetType.isDateTimeType()) {
+ return new DateTimeLiteral(value);
+ }
+ if (targetType.isDateTimeV2Type()) {
+ return new DateTimeV2Literal(value);
Review Comment:
why not create with type?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SinglePartitionDesc.java:
##########
@@ -262,8 +262,8 @@ public PartitionDefinition translateToPartitionDefinition()
{
} else if (partitionValue.isMax()) {
return
PartitionDefinition.MaxValue.INSTANCE;
} else {
- String stringValue =
partitionValue.getStringValue();
- return Literal.of(stringValue);
+ return Literal.fromLegacyLiteral(
Review Comment:
这个 `Literal.fromLegacyLiteral` 函数，能把第二个参数去掉吗？
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java:
##########
@@ -189,12 +200,73 @@ 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));
+ }
+
+ static Expression strictTypedPartitionExpression(Expression expression,
DataType targetType) {
+ if (expression instanceof MaxValue || expression instanceof
MaxLiteral) {
+ return expression;
+ }
+ if (expression.isNullLiteral()) {
+ return expression.checkedCastTo(targetType);
+ }
+ if (!expression.isLiteral()) {
+ throw new AnalysisException("Partition value must be literal: " +
expression.toSql());
+ }
+ String value = ((Literal) expression).getStringValue();
+ try {
+ if (targetType.isDateTimeType() || targetType.isDateTimeV2Type()
|| targetType.isTimeStampTzType()) {
+ return convertPartitionLiteral(value,
targetType).checkedCastTo(targetType);
+ }
+ validateCharacterLength(value, targetType);
+ LiteralExpr typedLiteral = LiteralExprUtils.createLiteral(
+ value, targetType.toCatalogDataType());
+ return Literal.fromLegacyLiteral(typedLiteral,
typedLiteral.getType());
Review Comment:
这一段好绕，需要重新整理一下
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java:
##########
@@ -47,6 +46,10 @@ public void validate(Map<String, String> properties) {
} catch (Exception e) {
throw new AnalysisException(e.getMessage(), e.getCause());
}
+ validateValueCount();
+ for (int i = 0; i < values.size(); ++i) {
+ typedPartitionExpression(values.get(i), i);
Review Comment:
能在 PartitionDefinition 基类上抽象个接口，然后在一个地方一次性转换完成吗？
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AlterMultiPartitionOp.java:
##########
@@ -89,18 +91,27 @@ public Map<String, String> getProperties() {
* getPartitionKeyDesc
*/
public PartitionKeyDesc getPartitionKeyDesc() {
- List<PartitionValue> fromValues = fromExpression.stream()
- .map(this::toLegacyPartitionValue)
- .collect(Collectors.toList());
- List<PartitionValue> toValues = toExpression.stream()
- .map(this::toLegacyPartitionValue)
- .collect(Collectors.toList());
+ validateBoundaryValueCount();
+ List<PartitionValue> fromValues = new ArrayList<>();
+ for (int i = 0; i < fromExpression.size(); i++) {
+ Expression typedFrom = typedExpression(fromExpression.get(i), i);
+ fromValues.add(toLegacyPartitionValue(typedFrom));
Review Comment:
现在的代码，看起来需要在各个地方散落的调用，将 partition definition 中的 expression
转换到对的类型，一旦有地方漏了，就会出问题。得想想怎么能够优雅的解决这个问题
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionExprDateTrunc.java:
##########
@@ -152,14 +155,17 @@ private PartitionValue getUpperValue(PartitionValue
upperValue, DateTimeV2Litera
throw new AnalysisException("date trunc not support MAXVALUE
partition");
}
// begin time and end time dateTrunc should has same result
- DateTimeV2Literal endTruncTime =
dateTrunc(upperValue.getStringValue(), Optional.empty(), true);
+ DateTimeV2Literal endTruncTime =
dateTrunc(upperValue.getValue().getStringValue(), Optional.empty(), true);
Review Comment:
这里的 date trunc 可以直接走表达式么？
--
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]
