database/doris
2
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

[fix](Nereids) create double literal when create decimal literal failed (#28959)

Browse Source 
FIX
1. remove float and double literal toString and getStringValue introduced by
  PR #23504 and PR #23271
  These functions lead to wrong cast result of double and float literal
2. fix compute signature for datetimev2 always produce scale 6
3. fix stats calculator failed when generate node stats with two same column
4. constant fold on fe failed when cast double to integral

TODO
after fix the first problem, some mv matching not work well, fix them later
- test_dup_mv_div
- test_dup_mv_json
- test_tcu
This commit is contained in:
morrySnow
2024-01-09 16:37:58 +08:00
committed by yiguolei
parent 67986a8a6f
commit 9cbb55d49b
31 changed files with 122 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
View File

@ -308,6 +308,7 @@ import org.apache.doris.nereids.trees.expressions.literal.DateTimeV2Literal;
import org.apache.doris.nereids.trees.expressions.literal.DateV2Literal;
import org.apache.doris.nereids.trees.expressions.literal.DecimalLiteral;
import org.apache.doris.nereids.trees.expressions.literal.DecimalV3Literal;
import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
import org.apache.doris.nereids.trees.expressions.literal.Interval;
import org.apache.doris.nereids.trees.expressions.literal.LargeIntLiteral;
@ -3057,10 +3058,14 @@ public class LogicalPlanBuilder extends DorisParserBaseVisitor<Object> {
@Override
public Literal visitDecimalLiteral(DecimalLiteralContext ctx) {
if (Config.enable_decimal_conversion) {
return new DecimalV3Literal(new BigDecimal(ctx.getText()));
} else {
return new DecimalLiteral(new BigDecimal(ctx.getText()));
try {
if (Config.enable_decimal_conversion) {
return new DecimalV3Literal(new BigDecimal(ctx.getText()));
} else {
return new DecimalLiteral(new BigDecimal(ctx.getText()));
}
} catch (Exception e) {
return new DoubleLiteral(Double.parseDouble(ctx.getText()));
}
}

8
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java
View File

@ -787,7 +787,7 @@ public class StatsCalculator extends DefaultPlanVisitor<Statistics, Void> {
.setNumNulls(stats.numNulls < 0 ? stats.numNulls : stats.numNulls * groupingSetNum)
.setDataSize(stats.dataSize < 0 ? stats.dataSize : stats.dataSize * groupingSetNum);
return Pair.of(kv.getKey(), columnStatisticBuilder.build());
}).collect(Collectors.toMap(Pair::key, Pair::value));
}).collect(Collectors.toMap(Pair::key, Pair::value, (item1, item2) -> item1));
return new Statistics(rowCount < 0 ? rowCount : rowCount * groupingSetNum, 1, columnStatisticMap);
}
@ -808,7 +808,7 @@ public class StatsCalculator extends DefaultPlanVisitor<Statistics, Void> {
// TODO: compute the literal size
return Pair.of(project.toSlot(), statistic);
})
.collect(Collectors.toMap(Pair::key, Pair::value));
.collect(Collectors.toMap(Pair::key, Pair::value, (item1, item2) -> item1));
int rowCount = 1;
return new Statistics(rowCount, 1, columnStatsMap);
}
@ -823,7 +823,7 @@ public class StatsCalculator extends DefaultPlanVisitor<Statistics, Void> {
.setAvgSizeByte(0);
return Pair.of(project.toSlot(), columnStat.build());
})
.collect(Collectors.toMap(Pair::key, Pair::value));
.collect(Collectors.toMap(Pair::key, Pair::value, (item1, item2) -> item1));
int rowCount = 0;
return new Statistics(rowCount, 1, columnStatsMap);
}
@ -998,7 +998,7 @@ public class StatsCalculator extends DefaultPlanVisitor<Statistics, Void> {
}
}
return Pair.of(expr.toSlot(), colStatsBuilder.build());
}).collect(Collectors.toMap(Pair::key, Pair::value));
}).collect(Collectors.toMap(Pair::key, Pair::value, (item1, item2) -> item1));
columnStatisticMap.putAll(childColumnStats);
return new Statistics(childStats.getRowCount(), 1, columnStatisticMap);
}

3
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java
View File

@ -388,8 +388,7 @@ public class ComputeSignatureHelper {
if (finalType == null) {
finalType = dateTimeV2Type;
} else {
finalType = DateTimeV2Type.getWiderDatetimeV2Type(finalType,
DateTimeV2Type.forType(arguments.get(i).getDataType()));
finalType = DateTimeV2Type.getWiderDatetimeV2Type(finalType, dateTimeV2Type);
}
}
}

14
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DoubleLiteral.java
View File

@ -23,8 +23,6 @@ import org.apache.doris.catalog.Type;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.DoubleType;
import java.text.NumberFormat;
/**
* Double literal
*/
@ -51,16 +49,4 @@ public class DoubleLiteral extends Literal {
public LiteralExpr toLegacyLiteral() {
return new FloatLiteral(value, Type.DOUBLE);
}
@Override
public String toString() {
NumberFormat nf = NumberFormat.getInstance();
nf.setGroupingUsed(false);
return nf.format(value);
}
@Override
public String getStringValue() {
return toString();
}
}

9
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/FloatLiteral.java
View File

@ -22,8 +22,6 @@ import org.apache.doris.catalog.Type;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.FloatType;
import java.text.NumberFormat;
/**
* float type literal
*/
@ -50,11 +48,4 @@ public class FloatLiteral extends Literal {
public LiteralExpr toLegacyLiteral() {
return new org.apache.doris.analysis.FloatLiteral((double) value, Type.FLOAT);
}
@Override
public String getStringValue() {
NumberFormat nf = NumberFormat.getInstance();
nf.setGroupingUsed(false);
return nf.format(value);
}
}

8
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/Literal.java
View File

@ -34,6 +34,7 @@ import org.apache.doris.nereids.types.DecimalV3Type;
import org.apache.doris.nereids.types.LargeIntType;
import org.apache.doris.nereids.types.StringType;
import org.apache.doris.nereids.types.VarcharType;
import org.apache.doris.nereids.types.coercion.IntegralType;
import com.google.common.collect.ImmutableList;
@ -234,6 +235,13 @@ public abstract class Literal extends Expression implements LeafExpression, Comp
return Literal.of(true);
}
}
if (targetType instanceof IntegralType) {
// do trailing zeros to avoid number parse error when cast to integral type
BigDecimal bigDecimal = new BigDecimal(desc);
if (bigDecimal.stripTrailingZeros().scale() <= 0) {
desc = bigDecimal.stripTrailingZeros().toPlainString();
}
}
if (targetType.isTinyIntType()) {
return Literal.of(Byte.valueOf(desc));
} else if (targetType.isSmallIntType()) {

38
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/InsertExecutor.java
View File

@ -58,6 +58,7 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalInlineTable;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.util.RelationUtil;
import org.apache.doris.nereids.util.TypeCoercionUtils;
import org.apache.doris.planner.DataSink;
import org.apache.doris.planner.OlapTableSink;
import org.apache.doris.proto.InternalService;
@ -553,20 +554,22 @@ public class InsertExecutor {
throw new AnalysisException("Column count doesn't match value count");
}
for (int i = 0; i < values.size(); i++) {
Column sameNameColumn = null;
for (Column column : table.getBaseSchema(true)) {
if (unboundTableSink.getColNames().get(i).equalsIgnoreCase(column.getName())) {
sameNameColumn = column;
break;
}
}
if (sameNameColumn == null) {
throw new AnalysisException("Unknown column '"
+ unboundTableSink.getColNames().get(i) + "' in target table.");
}
if (values.get(i) instanceof DefaultValueSlot) {
boolean hasDefaultValue = false;
for (Column column : columns) {
if (unboundTableSink.getColNames().get(i).equalsIgnoreCase(column.getName())) {
constantExprs.add(generateDefaultExpression(column));
hasDefaultValue = true;
}
}
if (!hasDefaultValue) {
throw new AnalysisException("Unknown column '"
+ unboundTableSink.getColNames().get(i) + "' in target table.");
}
constantExprs.add(generateDefaultExpression(sameNameColumn));
} else {
constantExprs.add(values.get(i));
DataType targetType = DataType.fromCatalogType(sameNameColumn.getType());
constantExprs.add((NamedExpression) castValue(values.get(i), targetType));
}
}
} else {
@ -577,7 +580,8 @@ public class InsertExecutor {
if (values.get(i) instanceof DefaultValueSlot) {
constantExprs.add(generateDefaultExpression(columns.get(i)));
} else {
constantExprs.add(values.get(i));
DataType targetType = DataType.fromCatalogType(columns.get(i).getType());
constantExprs.add((NamedExpression) castValue(values.get(i), targetType));
}
}
}
@ -595,6 +599,14 @@ public class InsertExecutor {
}
}
private static Expression castValue(Expression value, DataType targetType) {
if (value instanceof UnboundAlias) {
return value.withChildren(TypeCoercionUtils.castUnbound(((UnboundAlias) value).child(), targetType));
} else {
return TypeCoercionUtils.castUnbound(value, targetType);
}
}
/**
* get target table from names.
*/

13
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java
View File

@ -422,6 +422,14 @@ public class TypeCoercionUtils {
}
}
public static Expression castUnbound(Expression expression, DataType targetType) {
if (expression instanceof Literal) {
return TypeCoercionUtils.castIfNotSameType(expression, targetType);
} else {
return TypeCoercionUtils.unSafeCast(expression, targetType);
}
}
/**
* like castIfNotSameType does, but varchar or char type would be cast to target length exactly
*/
@ -467,11 +475,6 @@ public class TypeCoercionUtils {
return promoted;
}
}
// adapt scale when from string to datetimev2 with float
if (type.isStringLikeType() && dataType.isDateTimeV2Type()) {
return recordTypeCoercionForSubQuery(input,
DateTimeV2Type.forTypeFromString(((Literal) input).getStringValue()));
}
}
return recordTypeCoercionForSubQuery(input, dataType);
}

18
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyArithmeticRuleTest.java
View File

@ -36,7 +36,7 @@ class SimplifyArithmeticRuleTest extends ExpressionRewriteTestHelper {
assertRewriteAfterTypeCoercion("IA", "IA");
assertRewriteAfterTypeCoercion("IA + 1", "IA + 1");
assertRewriteAfterTypeCoercion("IA + IB", "IA + IB");
assertRewriteAfterTypeCoercion("1 * 3 / IA", "(3 / cast(IA as DOUBLE))");
assertRewriteAfterTypeCoercion("1 * 3 / IA", "(3.0 / cast(IA as DOUBLE))");
assertRewriteAfterTypeCoercion("1 - IA", "1 - IA");
assertRewriteAfterTypeCoercion("1 + 1", "2");
assertRewriteAfterTypeCoercion("IA + 2 - 1", "IA + 1");
@ -44,12 +44,12 @@ class SimplifyArithmeticRuleTest extends ExpressionRewriteTestHelper {
assertRewriteAfterTypeCoercion("IA + 2 - ((1 - IB) - (3 + IC))", "IA + IB + IC + 4");
assertRewriteAfterTypeCoercion("IA * IB + 2 - IC * 2", "(IA * IB) - (IC * 2) + 2");
assertRewriteAfterTypeCoercion("IA * IB", "IA * IB");
assertRewriteAfterTypeCoercion("IA * IB / 2 * 2", "cast((IA * IB) as DOUBLE) / 1");
assertRewriteAfterTypeCoercion("IA * IB / (2 * 2)", "cast((IA * IB) as DOUBLE) / 4");
assertRewriteAfterTypeCoercion("IA * IB / (2 * 2)", "cast((IA * IB) as DOUBLE) / 4");
assertRewriteAfterTypeCoercion("IA * (IB / 2) * 2)", "cast(IA as DOUBLE) * cast(IB as DOUBLE) / 1");
assertRewriteAfterTypeCoercion("IA * (IB / 2) * (IC + 1))", "cast(IA as DOUBLE) * cast(IB as DOUBLE) * cast((IC + 1) as DOUBLE) / 2");
assertRewriteAfterTypeCoercion("IA * IB / 2 / IC * 2 * ID / 4", "(((cast((IA * IB) as DOUBLE) / cast(IC as DOUBLE)) * cast(ID as DOUBLE)) / 4)");
assertRewriteAfterTypeCoercion("IA * IB / 2 * 2", "cast((IA * IB) as DOUBLE) / 1.0");
assertRewriteAfterTypeCoercion("IA * IB / (2 * 2)", "cast((IA * IB) as DOUBLE) / 4.0");
assertRewriteAfterTypeCoercion("IA * IB / (2 * 2)", "cast((IA * IB) as DOUBLE) / 4.0");
assertRewriteAfterTypeCoercion("IA * (IB / 2) * 2)", "cast(IA as DOUBLE) * cast(IB as DOUBLE) / 1.0");
assertRewriteAfterTypeCoercion("IA * (IB / 2) * (IC + 1))", "cast(IA as DOUBLE) * cast(IB as DOUBLE) * cast((IC + 1) as DOUBLE) / 2.0");
assertRewriteAfterTypeCoercion("IA * IB / 2 / IC * 2 * ID / 4", "(((cast((IA * IB) as DOUBLE) / cast(IC as DOUBLE)) * cast(ID as DOUBLE)) / 4.0)");
}
@Test
@ -86,8 +86,8 @@ class SimplifyArithmeticRuleTest extends ExpressionRewriteTestHelper {
assertRewriteAfterTypeCoercion("IA + 1 > IB", "cast(IA as BIGINT) > (cast(IB as BIGINT) - 1)");
assertRewriteAfterTypeCoercion("IA + 1 > IB * IC", "cast(IA as BIGINT) > ((IB * IC) - 1)");
assertRewriteAfterTypeCoercion("IA * ID > IB * IC", "IA * ID > IB * IC");
assertRewriteAfterTypeCoercion("IA * ID / 2 > IB * IC", "cast((IA * ID) as DOUBLE) > cast((IB * IC) as DOUBLE) * 2");
assertRewriteAfterTypeCoercion("IA * ID / -2 > IB * IC", "cast((IB * IC) as DOUBLE) * -2 > cast((IA * ID) as DOUBLE)");
assertRewriteAfterTypeCoercion("IA * ID / 2 > IB * IC", "cast((IA * ID) as DOUBLE) > cast((IB * IC) as DOUBLE) * 2.0");
assertRewriteAfterTypeCoercion("IA * ID / -2 > IB * IC", "cast((IB * IC) as DOUBLE) * -2.0 > cast((IA * ID) as DOUBLE)");
assertRewriteAfterTypeCoercion("1 - IA > 1", "(cast(IA as BIGINT) < 0)");
assertRewriteAfterTypeCoercion("1 - IA + 1 * 3 - 5 > 1", "(cast(IA as BIGINT) < -2)");
}

14
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelperTest.java
View File

@ -389,17 +389,17 @@ public class ComputeSignatureHelperTest {
new NullLiteral(),
new DateTimeV2Literal("2020-02-02 00:00:00.1234"));
signature = ComputeSignatureHelper.computePrecision(new FakeComputeSignature(), signature, arguments);
Assertions.assertTrue(signature.getArgType(0) instanceof MapType);
Assertions.assertEquals(DateTimeV2Type.of(6),
Assertions.assertInstanceOf(MapType.class, signature.getArgType(0));
Assertions.assertEquals(DateTimeV2Type.of(4),
((MapType) signature.getArgType(0)).getKeyType());
Assertions.assertEquals(DateTimeV2Type.of(6),
Assertions.assertEquals(DateTimeV2Type.of(4),
((MapType) signature.getArgType(0)).getValueType());
Assertions.assertTrue(signature.getArgType(1) instanceof MapType);
Assertions.assertEquals(DateTimeV2Type.of(6),
Assertions.assertInstanceOf(MapType.class, signature.getArgType(1));
Assertions.assertEquals(DateTimeV2Type.of(4),
((MapType) signature.getArgType(1)).getKeyType());
Assertions.assertEquals(DateTimeV2Type.of(6),
Assertions.assertEquals(DateTimeV2Type.of(4),
((MapType) signature.getArgType(1)).getValueType());
Assertions.assertEquals(DateTimeV2Type.of(6),
Assertions.assertEquals(DateTimeV2Type.of(4),
signature.getArgType(2));
}