This is an automated email from the ASF dual-hosted git repository.
yiguolei 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 505758c76b [BUG] (decimalv3) fix FE UTs (#10834)
505758c76b is described below
commit 505758c76ba57f5e634788ba850c78925e8c39a0
Author: Gabriel <gabrielleeb...@gmail.com>
AuthorDate: Thu Jul 14 19:24:50 2022 +0800
[BUG] (decimalv3) fix FE UTs (#10834)
---
.../org/apache/doris/analysis/ArithmeticExpr.java | 2 +-
.../apache/doris/analysis/FunctionCallExpr.java | 24 +--
.../java/org/apache/doris/analysis/TypeDef.java | 20 +++
.../java/org/apache/doris/catalog/Function.java | 8 +-
.../java/org/apache/doris/catalog/ScalarType.java | 8 +-
.../apache/doris/analysis/ArithmeticExprTest.java | 175 ---------------------
.../doris/analysis/FunctionCallExprTest.java | 82 ----------
7 files changed, 46 insertions(+), 273 deletions(-)
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 a8abf3e97e..ca0342a4bd 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
@@ -273,7 +273,7 @@ public class ArithmeticExpr extends Expr {
@Override
protected void toThrift(TExprNode msg) {
msg.node_type = TExprNodeType.ARITHMETIC_EXPR;
- if (!(type.isDecimalV2() && type.isDecimalV3())) {
+ if (!(type.isDecimalV2() || type.isDecimalV3())) {
msg.setOpcode(op.getOpcode());
msg.setOutputColumn(outputColumn);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
index 2505a2501d..2e1edacea7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
@@ -1075,17 +1075,19 @@ public class FunctionCallExpr extends Expr {
this.type = fn.getReturnType();
}
- // DECIMAL need to pass precision and scale to be
- if (DECIMAL_FUNCTION_SET.contains(fn.getFunctionName().getFunction())
- && (this.type.isDecimalV2() || this.type.isDecimalV3())) {
- if (DECIMAL_SAME_TYPE_SET.contains(fnName.getFunction())) {
- this.type = argTypes[0];
- } else if (DECIMAL_WIDER_TYPE_SET.contains(fnName.getFunction())) {
- this.type =
ScalarType.createDecimalType(ScalarType.MAX_DECIMAL128_PRECISION,
- ((ScalarType) argTypes[0]).getScalarScale());
- } else if (STDDEV_FUNCTION_SET.contains(fnName.getFunction())) {
- // for all stddev function, use decimal(38,9) as computing
result
- this.type =
ScalarType.createDecimalType(ScalarType.MAX_DECIMAL128_PRECISION,
STDDEV_DECIMAL_SCALE);
+ if (this.type.isDecimalV3()) {
+ // DECIMAL need to pass precision and scale to be
+ if
(DECIMAL_FUNCTION_SET.contains(fn.getFunctionName().getFunction())
+ && (this.type.isDecimalV2() || this.type.isDecimalV3())) {
+ if (DECIMAL_SAME_TYPE_SET.contains(fnName.getFunction())) {
+ this.type = argTypes[0];
+ } else if
(DECIMAL_WIDER_TYPE_SET.contains(fnName.getFunction())) {
+ this.type =
ScalarType.createDecimalType(ScalarType.MAX_DECIMAL128_PRECISION,
+ ((ScalarType) argTypes[0]).getScalarScale());
+ } else if (STDDEV_FUNCTION_SET.contains(fnName.getFunction()))
{
+ // for all stddev function, use decimal(38,9) as computing
result
+ this.type =
ScalarType.createDecimalType(ScalarType.MAX_DECIMAL128_PRECISION,
STDDEV_DECIMAL_SCALE);
+ }
}
}
// rewrite return type if is nested type function
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/TypeDef.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/TypeDef.java
index 3e1d137e19..91ce563896 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/TypeDef.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/TypeDef.java
@@ -169,6 +169,11 @@ public class TypeDef implements ParseNode {
throw new AnalysisException(
"Scale of decimal must between 0 and 9." + " Scale
was set to: " + scale + ".");
}
+ // scale < precision
+ if (scale >= precision) {
+ throw new AnalysisException("Scale of decimal must be
smaller than precision."
+ + " Scale is " + scale + " and precision is " +
precision);
+ }
break;
}
case DECIMAL32: {
@@ -183,6 +188,11 @@ public class TypeDef implements ParseNode {
throw new AnalysisException(
"Scale of decimal must not be less than 0." + "
Scale was set to: " + decimal32Scale + ".");
}
+ // scale < precision
+ if (decimal32Scale >= decimal32Precision) {
+ throw new AnalysisException("Scale of decimal must be
smaller than precision."
+ + " Scale is " + decimal32Scale + " and precision
is " + decimal32Precision);
+ }
break;
}
case DECIMAL64: {
@@ -197,6 +207,11 @@ public class TypeDef implements ParseNode {
throw new AnalysisException(
"Scale of decimal must not be less than 0." + "
Scale was set to: " + decimal64Scale + ".");
}
+ // scale < precision
+ if (decimal64Scale >= decimal64Precision) {
+ throw new AnalysisException("Scale of decimal must be
smaller than precision."
+ + " Scale is " + decimal64Scale + " and precision
is " + decimal64Precision);
+ }
break;
}
case DECIMAL128: {
@@ -211,6 +226,11 @@ public class TypeDef implements ParseNode {
throw new AnalysisException("Scale of decimal must not be
less than 0." + " Scale was set to: "
+ decimal128Scale + ".");
}
+ // scale < precision
+ if (decimal128Scale >= decimal128Precision) {
+ throw new AnalysisException("Scale of decimal must be
smaller than precision."
+ + " Scale is " + decimal128Scale + " and precision
is " + decimal128Precision);
+ }
break;
}
case TIMEV2:
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Function.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/Function.java
index 500e709495..652b81519c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Function.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Function.java
@@ -460,7 +460,13 @@ public class Function implements Writable {
if (location != null) {
fn.setHdfsLocation(location.getLocation());
}
- fn.setArgTypes(Type.toThrift(Lists.newArrayList(argTypes),
Lists.newArrayList(realArgTypes)));
+ // `realArgTypes.length != argTypes.length` is true iff this is an
aggregation function.
+ // For aggregation functions, `argTypes` here is already its real type
with true precision and scale.
+ if (realArgTypes.length != argTypes.length) {
+ fn.setArgTypes(Type.toThrift(Lists.newArrayList(argTypes)));
+ } else {
+ fn.setArgTypes(Type.toThrift(Lists.newArrayList(argTypes),
Lists.newArrayList(realArgTypes)));
+ }
// For types with different precisions and scales, return type only
indicates a type with default
// precision and scale so we need to transform it to the correct type.
if
(PrimitiveType.typeWithPrecision.contains(realReturnType.getPrimitiveType())) {
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 26ba564769..1bf64eb011 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
@@ -719,7 +719,7 @@ public class ScalarType extends Type {
return true;
}
if (isDecimalV3() && scalarType.isDecimalV3()) {
- return true;
+ return precision == scalarType.precision && scale ==
scalarType.scale;
}
return false;
}
@@ -742,6 +742,9 @@ public class ScalarType extends Type {
if ((this.isDatetimeV2() && other.isDatetimeV2()) || (this.isTimeV2()
&& other.isTimeV2())) {
return this.decimalScale() == other.decimalScale();
}
+ if (type.isDecimalV3Type() && other.isDecimalV3()) {
+ return precision == other.precision && scale == other.scale;
+ }
if (type != other.type) {
return false;
}
@@ -751,8 +754,7 @@ public class ScalarType extends Type {
if (type == PrimitiveType.VARCHAR) {
return len == other.len;
}
- if (type.isDecimalV2Type() || type.isDecimalV3Type()
- || type == PrimitiveType.DATETIMEV2 || type ==
PrimitiveType.TIMEV2) {
+ if (type.isDecimalV2Type() || type == PrimitiveType.DATETIMEV2 || type
== PrimitiveType.TIMEV2) {
return precision == other.precision && scale == other.scale;
}
return true;
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/analysis/ArithmeticExprTest.java
b/fe/fe-core/src/test/java/org/apache/doris/analysis/ArithmeticExprTest.java
deleted file mode 100644
index d985a8a72c..0000000000
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/ArithmeticExprTest.java
+++ /dev/null
@@ -1,175 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package org.apache.doris.analysis;
-
-import org.apache.doris.catalog.PrimitiveType;
-import org.apache.doris.catalog.ScalarType;
-import org.apache.doris.common.AnalysisException;
-import org.apache.doris.common.util.VectorizedUtil;
-import org.apache.doris.datasource.InternalDataSource;
-
-import mockit.Expectations;
-import mockit.Mocked;
-import org.junit.Assert;
-import org.junit.Test;
-
-import java.util.Arrays;
-import java.util.List;
-
-public class ArithmeticExprTest {
- private static final String internalCtl =
InternalDataSource.INTERNAL_DS_NAME;
-
- @Test
- public void testDecimalArithmetic(@Mocked VectorizedUtil vectorizedUtil) {
- Expr lhsExpr = new SlotRef(new TableName(internalCtl, "db", "table"),
"c0");
- Expr rhsExpr = new SlotRef(new TableName(internalCtl, "db", "table"),
"c1");
- ScalarType t1;
- ScalarType t2;
- ScalarType res;
- ArithmeticExpr arithmeticExpr;
- boolean hasException = false;
- new Expectations() {
- {
- vectorizedUtil.isVectorized();
- result = true;
- }
- };
- List<ArithmeticExpr.Operator> operators =
Arrays.asList(ArithmeticExpr.Operator.ADD,
- ArithmeticExpr.Operator.SUBTRACT, ArithmeticExpr.Operator.MOD,
- ArithmeticExpr.Operator.MULTIPLY,
ArithmeticExpr.Operator.DIVIDE);
- try {
- for (ArithmeticExpr.Operator operator : operators) {
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createDecimalType(19, 6);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createDecimalType(38, 6);
- if (operator == ArithmeticExpr.Operator.MULTIPLY) {
- res = ScalarType.createDecimalType(38, 10);
- }
- if (operator == ArithmeticExpr.Operator.DIVIDE) {
- res = ScalarType.createDecimalType(38, 4);
- }
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
-
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createDecimalType(18, 5);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createDecimalType(18, 5);
- if (operator == ArithmeticExpr.Operator.MULTIPLY) {
- res = ScalarType.createDecimalType(18, 9);
- }
- if (operator == ArithmeticExpr.Operator.DIVIDE) {
- res = ScalarType.createDecimalType(18, 4);
- }
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
-
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createType(PrimitiveType.BIGINT);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createDecimalType(18, 4);
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
-
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createType(PrimitiveType.LARGEINT);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createDecimalType(38, 4);
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
-
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createType(PrimitiveType.INT);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createDecimalType(9, 4);
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
-
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createType(PrimitiveType.FLOAT);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createType(PrimitiveType.DOUBLE);
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
-
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createType(PrimitiveType.DOUBLE);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createType(PrimitiveType.DOUBLE);
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
- }
- } catch (AnalysisException e) {
- e.printStackTrace();
- hasException = true;
- }
- Assert.assertFalse(hasException);
- }
-
- @Test
- public void testDecimalBitOperation(@Mocked VectorizedUtil vectorizedUtil)
{
- Expr lhsExpr = new SlotRef(new TableName(internalCtl, "db", "table"),
"c0");
- Expr rhsExpr = new SlotRef(new TableName(internalCtl, "db", "table"),
"c1");
- ScalarType t1;
- ScalarType t2;
- ScalarType res;
- ArithmeticExpr arithmeticExpr;
- boolean hasException = false;
- new Expectations() {
- {
- vectorizedUtil.isVectorized();
- result = true;
- }
- };
- List<ArithmeticExpr.Operator> operators =
Arrays.asList(ArithmeticExpr.Operator.BITAND,
- ArithmeticExpr.Operator.BITOR, ArithmeticExpr.Operator.BITXOR);
- try {
- for (ArithmeticExpr.Operator operator : operators) {
- t1 = ScalarType.createDecimalType(9, 4);
- t2 = ScalarType.createDecimalType(19, 6);
- lhsExpr.setType(t1);
- rhsExpr.setType(t2);
- arithmeticExpr = new ArithmeticExpr(operator, lhsExpr,
rhsExpr);
- res = ScalarType.createType(PrimitiveType.BIGINT);
- arithmeticExpr.analyzeImpl(null);
- Assert.assertEquals(arithmeticExpr.type, res);
- Assert.assertTrue(arithmeticExpr.getChild(0) instanceof
CastExpr);
- Assert.assertTrue(arithmeticExpr.getChild(1) instanceof
CastExpr);
- }
- } catch (AnalysisException e) {
- e.printStackTrace();
- hasException = true;
- }
- Assert.assertFalse(hasException);
- }
-}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/analysis/FunctionCallExprTest.java
b/fe/fe-core/src/test/java/org/apache/doris/analysis/FunctionCallExprTest.java
deleted file mode 100644
index 6d8cbe7433..0000000000
---
a/fe/fe-core/src/test/java/org/apache/doris/analysis/FunctionCallExprTest.java
+++ /dev/null
@@ -1,82 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package org.apache.doris.analysis;
-
-import org.apache.doris.catalog.ScalarType;
-import org.apache.doris.catalog.Type;
-import org.apache.doris.common.AnalysisException;
-import org.apache.doris.datasource.InternalDataSource;
-
-import com.google.common.collect.ImmutableList;
-import mockit.Mock;
-import mockit.MockUp;
-import mockit.Mocked;
-import org.junit.Assert;
-import org.junit.Test;
-
-import java.util.Arrays;
-
-public class FunctionCallExprTest {
- private static final String internalCtl =
InternalDataSource.INTERNAL_DS_NAME;
-
- @Test
- public void testDecimalFunction(@Mocked Analyzer analyzer) throws
AnalysisException {
- new MockUp<SlotRef>(SlotRef.class) {
- @Mock
- public void analyzeImpl(Analyzer analyzer) throws
AnalysisException {
- return;
- }
- };
- Expr argExpr = new SlotRef(new TableName(internalCtl, "db", "table"),
"c0");
- FunctionCallExpr functionCallExpr;
- boolean hasException = false;
- Type res;
- ImmutableList<String> sameTypeFunction =
ImmutableList.<String>builder()
- .add("min").add("max").add("lead").add("lag")
- .add("first_value").add("last_value").add("abs")
- .add("positive").add("negative").build();
- ImmutableList<String> widerTypeFunction =
ImmutableList.<String>builder()
- .add("sum").add("avg").add("multi_distinct_sum").build();
- try {
- for (String func : sameTypeFunction) {
- Type argType = ScalarType.createDecimalType(9, 4);
- argExpr.setType(argType);
- functionCallExpr = new FunctionCallExpr(func,
Arrays.asList(argExpr));
- functionCallExpr.setIsAnalyticFnCall(true);
- res = ScalarType.createDecimalType(9, 4);
- functionCallExpr.analyzeImpl(analyzer);
- Assert.assertEquals(functionCallExpr.type, res);
- }
-
- for (String func : widerTypeFunction) {
- Type argType = ScalarType.createDecimalType(9, 4);
- argExpr.setType(argType);
- functionCallExpr = new FunctionCallExpr(func,
Arrays.asList(argExpr));
- res = ScalarType.createDecimalType(38, 4);
- functionCallExpr.analyzeImpl(analyzer);
- Assert.assertEquals(functionCallExpr.type, res);
- }
-
- } catch (AnalysisException e) {
- e.printStackTrace();
- hasException = true;
- }
- Assert.assertFalse(hasException);
- }
-
-}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
