You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by jn...@apache.org on 2016/08/09 04:26:23 UTC
drill git commit: DRILL-4704: Fix incorrect query result when decimal
is compared with int value
Repository: drill
Updated Branches:
refs/heads/master e02aa596f -> aaf220ffd
DRILL-4704: Fix incorrect query result when decimal is compared with int value
Make sure implicit cast function is provided with the correct precision value
for int/bigint input.
This version of the fix passes the unit test, but may increase the chance of
an overflow error, since the precision is not minimized, and thus it is more
likely to exceed that of the destination decimal. see DRILL-4834.
Close apache/drill#517
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/aaf220ff
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/aaf220ff
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/aaf220ff
Branch: refs/heads/master
Commit: aaf220ffdddaa329e38237d69b438f4e09d16a87
Parents: e02aa59
Author: Dave Oshinsky <da...@yahoo.com>
Authored: Thu Jun 9 18:08:01 2016 -0400
Committer: Jinfeng Ni <jn...@apache.org>
Committed: Mon Aug 8 20:44:38 2016 -0700
----------------------------------------------------------------------
.../exec/expr/ExpressionTreeMaterializer.java | 16 +++-
.../exec/store/parquet/TestFixedlenDecimal.java | 82 +++++++++++++++++++
.../resources/parquet/fixedlenDecimal.parquet | Bin 0 -> 11297 bytes
.../apache/drill/exec/util/DecimalUtility.java | 2 +
4 files changed, 98 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/aaf220ff/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
index daac31d..bece93d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
@@ -89,6 +89,7 @@ import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
+import org.apache.drill.exec.util.DecimalUtility;
public class ExpressionTreeMaterializer {
@@ -267,6 +268,17 @@ public class ExpressionTreeMaterializer {
return new BooleanOperator(op.getName(), args, op.getPosition());
}
+ private int computePrecision(LogicalExpression currentArg) {
+ int precision = currentArg.getMajorType().getPrecision();
+ if (currentArg.getMajorType().getMinorType() == MinorType.INT) {
+ precision = DecimalUtility.MAX_DIGITS_INT;
+ }
+ else if (currentArg.getMajorType().getMinorType() == MinorType.BIGINT) {
+ precision = DecimalUtility.MAX_DIGITS_BIGINT;
+ }
+ return precision;
+ }
+
@Override
public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupContext functionLookupContext) {
List<LogicalExpression> args = Lists.newArrayList();
@@ -313,7 +325,7 @@ public class ExpressionTreeMaterializer {
if (CoreDecimalUtility.isDecimalType(parmType)) {
// We are implicitly promoting a decimal type, set the required scale and precision
parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()).
- setScale(currentArg.getMajorType().getScale()).setPrecision(currentArg.getMajorType().getPrecision()).build();
+ setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build();
}
argsWithCast.add(addCastExpression(currentArg, parmType, functionLookupContext, errorCollector));
}
@@ -339,7 +351,7 @@ public class ExpressionTreeMaterializer {
if (CoreDecimalUtility.isDecimalType(parmType)) {
// We are implicitly promoting a decimal type, set the required scale and precision
parmType = MajorType.newBuilder().setMinorType(parmType.getMinorType()).setMode(parmType.getMode()).
- setScale(currentArg.getMajorType().getScale()).setPrecision(currentArg.getMajorType().getPrecision()).build();
+ setScale(currentArg.getMajorType().getScale()).setPrecision(computePrecision(currentArg)).build();
}
extArgsWithCast.add(addCastExpression(call.args.get(i), parmType, functionLookupContext, errorCollector));
}
http://git-wip-us.apache.org/repos/asf/drill/blob/aaf220ff/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestFixedlenDecimal.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestFixedlenDecimal.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestFixedlenDecimal.java
new file mode 100644
index 0000000..124a8c8
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestFixedlenDecimal.java
@@ -0,0 +1,82 @@
+/**
+ * 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.drill.exec.store.parquet;
+
+import org.apache.drill.BaseTestQuery;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.proto.UserBitShared;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class TestFixedlenDecimal extends BaseTestQuery {
+ // enable decimal data type
+ @BeforeClass
+ public static void enableDecimalDataType() throws Exception {
+ test(String.format("alter session set `%s` = true", PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY));
+ }
+
+ private static final String DATAFILE = "cp.`parquet/fixedlenDecimal.parquet`";
+
+ @Test
+ public void testNullCount() throws Exception {
+ String query = String.format("select count(*) as c from %s where department_id is null", DATAFILE);
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(1L)
+ .build()
+ .run();
+ }
+
+ @Test
+ public void testNotNullCount() throws Exception {
+ String query = String.format("select count(*) as c from %s where department_id is not null", DATAFILE);
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(106L)
+ .build()
+ .run();
+ }
+
+ @Test
+ public void testSimpleQueryWithCast() throws Exception {
+ String query = String.format("select cast(department_id as bigint) as c from %s where cast(employee_id as decimal) = 170", DATAFILE);
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(80L)
+ .build()
+ .run();
+ }
+
+ @Test
+ public void testSimpleQueryDrill4704Fix() throws Exception {
+ String query = String.format("select cast(department_id as bigint) as c from %s where employee_id = 170", DATAFILE);
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(80L)
+ .build()
+ .run();
+ }
+}
http://git-wip-us.apache.org/repos/asf/drill/blob/aaf220ff/exec/java-exec/src/test/resources/parquet/fixedlenDecimal.parquet
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/parquet/fixedlenDecimal.parquet b/exec/java-exec/src/test/resources/parquet/fixedlenDecimal.parquet
new file mode 100644
index 0000000..42b474b
Binary files /dev/null and b/exec/java-exec/src/test/resources/parquet/fixedlenDecimal.parquet differ
http://git-wip-us.apache.org/repos/asf/drill/blob/aaf220ff/exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java
----------------------------------------------------------------------
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java b/exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java
index e8130ec..a87fe4c 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java
@@ -33,6 +33,8 @@ import org.apache.drill.exec.expr.holders.Decimal38SparseHolder;
public class DecimalUtility extends CoreDecimalUtility{
public final static int MAX_DIGITS = 9;
+ public final static int MAX_DIGITS_INT = 10;
+ public final static int MAX_DIGITS_BIGINT = 19;
public final static int DIGITS_BASE = 1000000000;
public final static int DIGITS_MAX = 999999999;
public final static int INTEGER_SIZE = (Integer.SIZE/8);