You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by li...@apache.org on 2023/01/06 02:39:00 UTC
[doris] branch master updated: [Bug](decimalv3) fix wrong result for MOD operation (#15644)
This is an automated email from the ASF dual-hosted git repository.
lihaopeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new b57500d0c3 [Bug](decimalv3) fix wrong result for MOD operation (#15644)
b57500d0c3 is described below
commit b57500d0c3a9ba59f3006ddca927520f9d46354a
Author: Gabriel <ga...@gmail.com>
AuthorDate: Fri Jan 6 10:38:53 2023 +0800
[Bug](decimalv3) fix wrong result for MOD operation (#15644)
---
be/src/vec/core/block.h | 4 +++-
.../java/org/apache/doris/analysis/ArithmeticExpr.java | 6 ++++++
.../java/org/apache/doris/analysis/BinaryPredicate.java | 7 ++++---
.../java/org/apache/doris/analysis/FloatLiteral.java | 4 ++--
.../org/apache/doris/analysis/SetOperationStmt.java | 6 ++++++
.../main/java/org/apache/doris/catalog/ScalarType.java | 8 +++++++-
.../src/main/java/org/apache/doris/catalog/Type.java | 6 ++----
.../main/java/org/apache/doris/planner/ScanNode.java | 5 ++++-
.../doris/analysis/CreateTableAsSelectStmtTest.java | 17 ++++++++++++-----
.../java/org/apache/doris/analysis/QueryStmtTest.java | 6 +-----
10 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/be/src/vec/core/block.h b/be/src/vec/core/block.h
index b4a73a896f..220a0ff361 100644
--- a/be/src/vec/core/block.h
+++ b/be/src/vec/core/block.h
@@ -459,7 +459,9 @@ public:
DCHECK_EQ(_columns.size(), block.columns());
for (int i = 0; i < _columns.size(); ++i) {
if (!_data_types[i]->equals(*block.get_by_position(i).type)) {
- DCHECK(_data_types[i]->is_nullable());
+ DCHECK(_data_types[i]->is_nullable())
+ << " target type: " << _data_types[i]->get_name()
+ << " src type: " << block.get_by_position(i).type->get_name();
DCHECK(((DataTypeNullable*)_data_types[i].get())
->get_nested_type()
->equals(*block.get_by_position(i).type));
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
index 99b6e39e3b..02ea5579cb 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
@@ -533,6 +533,9 @@ public class ArithmeticExpr extends Expr {
// max(scale1, scale2))
scale = Math.max(t1Scale, t2Scale);
precision = Math.max(widthOfIntPart1, widthOfIntPart2) + scale;
+ } else {
+ scale = Math.max(t1Scale, t2Scale);
+ precision = widthOfIntPart2 + scale;
}
if (precision > ScalarType.MAX_DECIMAL128_PRECISION) {
// TODO(gabriel): if precision is bigger than 38?
@@ -557,6 +560,9 @@ public class ArithmeticExpr extends Expr {
break;
}
castChild(ScalarType.createDecimalV3Type(precision, targetScale), 0);
+ } else if (op == Operator.MOD) {
+ castChild(type, 0);
+ castChild(type, 1);
}
break;
case INT_DIVIDE:
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
index bd2b8245bd..98827adc06 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
@@ -365,9 +365,6 @@ public class BinaryPredicate extends Predicate implements Writable {
if (t1 == PrimitiveType.BIGINT && t2 == PrimitiveType.BIGINT) {
return Type.getAssignmentCompatibleType(getChild(0).getType(), getChild(1).getType(), false);
}
- if (t1.isDecimalV3Type() || t2.isDecimalV3Type()) {
- return Type.getAssignmentCompatibleType(getChild(0).getType(), getChild(1).getType(), false);
- }
if ((t1 == PrimitiveType.BIGINT || t1 == PrimitiveType.DECIMALV2)
&& (t2 == PrimitiveType.BIGINT || t2 == PrimitiveType.DECIMALV2)) {
return Type.DECIMALV2;
@@ -393,6 +390,10 @@ public class BinaryPredicate extends Predicate implements Writable {
}
}
+ if ((t1.isDecimalV3Type() && !t2.isStringType()) || (t2.isDecimalV3Type() && !t1.isStringType())) {
+ return Type.getAssignmentCompatibleType(getChild(0).getType(), getChild(1).getType(), false);
+ }
+
return Type.DOUBLE;
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/FloatLiteral.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/FloatLiteral.java
index b6b9fb14fe..e62964c2d1 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FloatLiteral.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FloatLiteral.java
@@ -194,8 +194,8 @@ public class FloatLiteral extends LiteralExpr {
return new DecimalLiteral(BigDecimal.valueOf(value));
} else if (targetType.isDecimalV3()) {
DecimalLiteral res = new DecimalLiteral(new BigDecimal(value));
- res.setType(ScalarType.createDecimalV3Type(res.getType().getPrecision(),
- ((ScalarType) res.getType()).decimalScale()));
+ res.setType(ScalarType.createDecimalV3Type(targetType.getPrecision(),
+ ((ScalarType) targetType).decimalScale()));
return res;
}
return this;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java
index b599d2cb98..984f2c5822 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java
@@ -492,6 +492,12 @@ public class SetOperationStmt extends QueryStmt {
(ScalarType) selectTypeWithNullable.get(j).first,
(ScalarType) operands.get(i).getQueryStmt().getResultExprs().get(j).getType());
}
+ if (selectTypeWithNullable.get(j).first.isDecimalV3()
+ && operands.get(i).getQueryStmt().getResultExprs().get(j).getType().isDecimalV3()) {
+ selectTypeWithNullable.get(j).first = ScalarType.getAssignmentCompatibleDecimalV3Type(
+ (ScalarType) selectTypeWithNullable.get(j).first,
+ (ScalarType) operands.get(i).getQueryStmt().getResultExprs().get(j).getType());
+ }
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ScalarType.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ScalarType.java
index 446c73f58e..248925d990 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ScalarType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ScalarType.java
@@ -1051,7 +1051,7 @@ public class ScalarType extends Type {
if (t1.isDatetimeV2() && t2.isDatetimeV2()) {
return t1.scale > t2.scale ? t1 : t2;
}
- if ((t1.isDatetimeV2() || t1.isDateV2()) && (t1.isDatetimeV2() || t1.isDateV2())) {
+ if ((t1.isDatetimeV2() || t1.isDateV2()) && (t2.isDatetimeV2() || t2.isDateV2())) {
return t1.isDatetimeV2() ? t1 : t2;
}
if (strict) {
@@ -1074,6 +1074,12 @@ public class ScalarType extends Type {
targetPrecision, targetScale);
}
+ public static ScalarType getAssignmentCompatibleDecimalV3Type(ScalarType t1, ScalarType t2) {
+ int targetPrecision = Math.max(t1.decimalPrecision(), t2.decimalPrecision());
+ int targetScale = Math.max(t1.decimalScale(), t2.decimalScale());
+ return ScalarType.createDecimalV3Type(targetPrecision, targetScale);
+ }
+
/**
* Returns true t1 can be implicitly cast to t2, false otherwise.
* If strict is true, only consider casts that result in no loss of precision.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java
index 8c5517d01c..cffcc3a597 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java
@@ -1509,17 +1509,15 @@ public abstract class Type {
case DATE:
case DATEV2:
case DATETIME:
+ case DATETIMEV2:
case TIME:
+ case TIMEV2:
case CHAR:
case VARCHAR:
case HLL:
case BITMAP:
case QUANTILE_STATE:
return VARCHAR;
- case DATETIMEV2:
- return DEFAULT_DATETIMEV2;
- case TIMEV2:
- return DEFAULT_TIMEV2;
case DECIMALV2:
return DECIMALV2;
case DECIMAL32:
diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java
index 7733917087..c7031283da 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/ScanNode.java
@@ -105,7 +105,10 @@ public abstract class ScanNode extends PlanNode {
protected Expr castToSlot(SlotDescriptor slotDesc, Expr expr) throws UserException {
PrimitiveType dstType = slotDesc.getType().getPrimitiveType();
PrimitiveType srcType = expr.getType().getPrimitiveType();
- if (dstType != srcType) {
+ if (PrimitiveType.typeWithPrecision.contains(dstType) && PrimitiveType.typeWithPrecision.contains(srcType)
+ && !slotDesc.getType().equals(expr.getType())) {
+ return expr.castTo(slotDesc.getType());
+ } else if (dstType != srcType) {
return expr.castTo(slotDesc.getType());
} else {
return expr;
diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableAsSelectStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableAsSelectStmtTest.java
index de0f65f180..2a6bc8e8c6 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableAsSelectStmtTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableAsSelectStmtTest.java
@@ -103,12 +103,19 @@ public class CreateTableAsSelectStmtTest extends TestWithFeService {
createTableAsSelect(selectFromDecimal1);
if (Config.enable_decimal_conversion) {
Assertions.assertEquals(
- "CREATE TABLE `select_decimal_table_1` (\n" + " `_col0` decimal(38, 2) NULL\n" + ") ENGINE=OLAP\n"
- + "DUPLICATE KEY(`_col0`)\n" + "COMMENT 'OLAP'\n"
- + "DISTRIBUTED BY HASH(`_col0`) BUCKETS 10\n" + "PROPERTIES (\n"
+ "CREATE TABLE `select_decimal_table_1` (\n"
+ + " `_col0` decimal(38, 2) NULL\n"
+ + ") ENGINE=OLAP\n"
+ + "DUPLICATE KEY(`_col0`)\n"
+ + "COMMENT 'OLAP'\n"
+ + "DISTRIBUTED BY HASH(`_col0`) BUCKETS 10\n"
+ + "PROPERTIES (\n"
+ "\"replication_allocation\" = \"tag.location.default: 1\",\n"
- + "\"in_memory\" = \"false\",\n" + "\"storage_format\" = \"V2\","
- + "\n\"disable_auto_compaction\" = \"false\"\n" + ");",
+ + "\"in_memory\" = \"false\",\n"
+ + "\"storage_format\" = \"V2\",\n"
+ + "\"light_schema_change\" = \"true\",\n"
+ + "\"disable_auto_compaction\" = \"false\"\n"
+ + ");",
showCreateTableByName("select_decimal_table_1").getResultRows().get(0).get(1));
} else {
Assertions.assertEquals(
diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java
index e0b8a2f9b0..130c23d427 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java
@@ -190,11 +190,7 @@ public class QueryStmtTest {
Assert.assertEquals(2, exprsMap.size());
constMap.clear();
constMap = getConstantExprMap(exprsMap, analyzer);
- if (Config.enable_decimal_conversion) {
- Assert.assertEquals(6, constMap.size());
- } else {
- Assert.assertEquals(0, constMap.size());
- }
+ Assert.assertEquals(0, constMap.size());
sql = "SELECT k1 FROM db1.baseall GROUP BY k1 HAVING EXISTS(SELECT k4 FROM db1.tbl1 GROUP BY k4 "
+ "HAVING SUM(k4) = k4);";
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org