You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by vvysotskyi <gi...@git.apache.org> on 2018/04/20 14:39:32 UTC
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
GitHub user vvysotskyi opened a pull request:
https://github.com/apache/drill/pull/1232
DRILL-6094: Decimal data type enhancements
For details please see [design document](https://docs.google.com/document/d/1kfWUZ_OsEmLOJu_tKZO0-ZUJwc52drrm15fyif7BmzQ/edit?usp=sharing)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vvysotskyi/drill DRILL-6094
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1232.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1232
----
commit a276870d6bad293da0311aed746fb68a2268700b
Author: Dave Oshinsky <da...@...>
Date: 2016-02-09T22:37:47Z
DRILL-4184: Support variable length decimal fields in parquet
commit 84a31b92dc86873d14a7be601d189bb1cbd59b7d
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-05T12:35:42Z
DRILL-6094: Decimal data type enhancements
Add ExprVisitors for VARDECIMAL
commit e93cd93211ffa84586ee0d21dab26b128d81b290
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-05T14:28:39Z
DRILL-6094: Modify writers/readers to support VARDECIMAL
- Added usage of VarDecimal for parquet, hive, maprdb, jdbc;
- Added options to store decimals as int32 and int64 or fixed_len_byte_array or binary;
commit 8690897a4a2820f4e965cb562107782f1e57a99a
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-03T13:27:25Z
DRILL-6094: Refresh protobuf C++ source files
commit fbe93c46ed8a7e85a95da3ae145bfc00b7f17dd0
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-05T14:44:05Z
DRILL-6094: Add UDFs for VARDECIMAL data type
- modify type inference rules
- remove UDFs for obsolete DECIMAL types
commit 746e54f71464a9dea91c3860ba0346e40c1d6ddf
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-05T14:48:31Z
DRILL-6094: Enable DECIMAL data type by default
commit 46ce3d1504618ae7e547479280f5d60b35ec1caa
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-05T14:51:39Z
DRILL-6094: Add unit tests for DECIMAL data type
commit f62545390a37425b9fbb21881e4cd12ade337e73
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-05T14:58:36Z
DRILL-6094: Changes in C++ files
commit 7a04309605c0cd869a9eb55450aaa43c0088b425
Author: Volodymyr Vysotskyi <vv...@...>
Date: 2018-04-10T12:55:17Z
DRILL-6094: Fix mapping for NLJ when literal with non-primitive type is used in join conditions
----
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184008307
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java ---
@@ -228,14 +232,33 @@ private void newSchema() throws IOException {
setUp(schema, consumer);
}
- private PrimitiveType getPrimitiveType(MaterializedField field) {
+ protected PrimitiveType getPrimitiveType(MaterializedField field) {
MinorType minorType = field.getType().getMinorType();
String name = field.getName();
+ int length = ParquetTypeHelper.getLengthForMinorType(minorType);
PrimitiveTypeName primitiveTypeName = ParquetTypeHelper.getPrimitiveTypeNameForMinorType(minorType);
+ if (DecimalUtility.isDecimalType(minorType)) {
+ OptionManager optionManager = oContext.getFragmentContext().getOptions();
+ if (optionManager.getString(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS)
--- End diff --
Agree, thanks for pointing this, used `writerOptions`.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183756898
--- Diff: exec/java-exec/src/main/codegen/templates/Decimal/CastDecimalFloat.java ---
@@ -86,24 +46,21 @@ public void eval() {
*/
@SuppressWarnings("unused")
-@FunctionTemplate(name = "cast${type.to?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.NULL_IF_NULL)
+@FunctionTemplate(name = "cast${type.to?upper_case}",
+ scope = FunctionTemplate.FunctionScope.SIMPLE,
+ nulls=NullHandling.NULL_IF_NULL)
--- End diff --
` nulls=NullHandling.NULL_IF_NULL` -> `nulls = NullHandling.NULL_IF_NULL`
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184008988
--- Diff: exec/vector/src/main/codegen/templates/NullReader.java ---
@@ -31,19 +31,19 @@
* This class is generated using freemarker and the ${.template_name} template.
*/
@SuppressWarnings("unused")
-public class NullReader extends AbstractBaseReader implements FieldReader{
+public class NullReader extends AbstractBaseReader implements FieldReader {
public static final NullReader INSTANCE = new NullReader();
public static final NullReader EMPTY_LIST_INSTANCE = new NullReader(Types.repeated(TypeProtos.MinorType.NULL));
public static final NullReader EMPTY_MAP_INSTANCE = new NullReader(Types.required(TypeProtos.MinorType.MAP));
private MajorType type;
- private NullReader(){
+ private NullReader() {
--- End diff --
Thanks, removed.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183762729
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/stat/RangeExprEvaluator.java ---
@@ -219,6 +219,7 @@ private Statistics evalCastFunc(FunctionHolderExpression holderExpr, Statistics
return null; // cast func between srcType and destType is NOT allowed.
}
+ // TODO: add decimal support
--- End diff --
Please remove.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184031285
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java ---
@@ -382,13 +407,26 @@ public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
final RelDataType operandType = opBinding.getOperandType(0);
final TypeProtos.MinorType inputMinorType = getDrillTypeFromCalciteType(operandType);
- if(TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.BIGINT))
+ if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.BIGINT))
== TypeProtos.MinorType.BIGINT) {
return createCalciteTypeWithNullability(
factory,
SqlTypeName.BIGINT,
isNullable);
- } else if(TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT8))
+ } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT4))
--- End diff --
Thanks, added an explanation for sum and avg return type inferences.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183761333
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Hash64WithSeedAsDouble.java ---
@@ -265,6 +268,42 @@ public void eval() {
}
}
+ @FunctionTemplate(name = "hash64AsDouble", scope = FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+ public static class VarDecimalHash implements DrillSimpleFunc {
+ @Param VarDecimalHolder in;
+ @Param BigIntHolder seed;
+ @Output BigIntHolder out;
+
+ public void setup() {
+ }
+
+ public void eval() {
+ java.math.BigDecimal input = org.apache.drill.exec.util.DecimalUtility.getBigDecimalFromDrillBuf(in.buffer,
+ in.start, in.end - in.start, in.scale);
+ out.value = org.apache.drill.exec.expr.fn.impl.HashHelper.hash64(input.doubleValue(), seed.value);
+ }
+ }
+
+ @FunctionTemplate(name = "hash64AsDouble", scope = FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+ public static class NullableVarDecimalHash implements DrillSimpleFunc {
+ @Param NullableVarDecimalHolder in;
+ @Param BigIntHolder seed;
+ @Output BigIntHolder out;
+
+ public void setup() {
+ }
+
+ public void eval() {
+ if (in.isSet == 0) {
+ out.value = seed.value;
+ } else {
+ java.math.BigDecimal input = org.apache.drill.exec.util.DecimalUtility.getBigDecimalFromDrillBuf(in.buffer,
+ in.start, in.end - in.start, in.scale);
+ out.value = org.apache.drill.exec.expr.fn.impl.HashHelper.hash64(input.doubleValue(), seed.value);
+ }
+ }
+ }
--- End diff --
Please consider removing functions using old decimals.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183998600
--- Diff: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcRecordReader.java ---
@@ -225,10 +247,10 @@ public int next() {
int counter = 0;
Boolean b = true;
try {
- while (counter < 4095 && b == true) { // loop at 4095 since nullables use one more than record count and we
+ while (counter < 4095 && b) { // loop at 4095 since nullables use one more than record count and we
// allocate on powers of two.
b = resultSet.next();
- if(b == false) {
+ if(!b) {
--- End diff --
Thanks, done.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184031484
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/stat/ParquetMetaStatCollector.java ---
@@ -152,6 +152,7 @@ private ColumnStatistics getStat(Object min, Object max, Long numNull,
}
if (min != null && max != null ) {
+ // TODO: add vardecimal type
--- End diff --
Thanks, removed.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184061116
--- Diff: exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java ---
@@ -20,12 +20,17 @@
import java.io.IOException;
import java.net.URL;
+import com.google.common.base.Function;
--- End diff --
I tested it manually. I considered 2 options:
1. UDF has an input old decimal type. In this case, function resolver adds cast from vardecimal to old decimal type which is used in UDF.
2. UDF returns the old decimal type. In this case, Drill casts the result of UDF to vardecimal.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183358609
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestVarlenDecimal.java ---
@@ -17,61 +17,137 @@
*/
package org.apache.drill.exec.store.parquet;
+import org.apache.commons.io.FileUtils;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.test.BaseTestQuery;
import org.apache.drill.exec.planner.physical.PlannerSettings;
-import org.apache.drill.exec.proto.UserBitShared;
-import org.junit.BeforeClass;
-import org.junit.AfterClass;
+import org.hamcrest.CoreMatchers;
+import org.junit.Assert;
import org.junit.Test;
-public class TestVarlenDecimal extends BaseTestQuery {
- // enable decimal data type
- @BeforeClass
--- End diff --
Why did you remove before and after class? Instead of you are setting `alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);` in tests...
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183779315
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestVarlenDecimal.java ---
@@ -0,0 +1,153 @@
+/*
+ * 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.commons.io.FileUtils;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.hamcrest.CoreMatchers;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.math.BigDecimal;
+import java.nio.file.Paths;
+
+public class TestVarlenDecimal extends BaseTestQuery {
+
+ private static final String DATAFILE = "cp.`parquet/varlenDecimal.parquet`";
+
+ @Test
+ public void testNullCount() throws Exception {
+ String query = String.format("select count(*) as c from %s where department_id is null", DATAFILE);
--- End diff --
`sqlQuery` can do` String.format` for you.
---
[GitHub] drill issue #1232: DRILL-6094: Decimal data type enhancements
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:
https://github.com/apache/drill/pull/1232
+1 for the C++ part. Looks really good.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184008881
--- Diff: exec/vector/src/main/codegen/templates/AbstractPromotableFieldWriter.java ---
@@ -75,12 +75,19 @@ public void endList() {
<#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
<#assign fields = minor.fields!type.fields />
- <#if !minor.class?starts_with("Decimal") >
+ <#if minor.class?contains("VarDecimal")>
+ @Override
+ public void write${minor.class}(BigDecimal value) {
+ getWriter(MinorType.${name?upper_case}).write${minor.class}(value);
+ }
+ </#if>
+
@Override
public void write(${name}Holder holder) {
getWriter(MinorType.${name?upper_case}).write(holder);
}
+ <#if !minor.class?contains("Decimal") || minor.class?contains("VarDecimal")>
--- End diff --
Thanks, added.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184004929
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ---
@@ -409,7 +409,7 @@ public void clear() {
// Check if the field exists.
ValueVector v = fieldVectorMap.get(field.getName());
- if (v == null || v.getClass() != clazz) {
+ if (v == null || !v.getField().getType().equals(field.getType())) {
--- End diff --
Thanks, added a comment with the explanation.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184012035
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java ---
@@ -409,4 +409,30 @@ public void testNLJoinCorrectnessRightMultipleBatches() throws Exception {
setSessionOption(ExecConstants.SLICE_TARGET, 100000);
}
}
+
+ @Test
+ public void testNlJoinWithStringsInCondition() throws Exception {
+ try {
+ test(DISABLE_NLJ_SCALAR);
+ test(DISABLE_JOIN_OPTIMIZATION);
+
+ final String query =
--- End diff --
Before my changes similar query but with decimal literal in condition passed because it was handled as double. But since with my changes decimal literal is handled as the decimal, it is discovered a bug in nested loop join which is observed for the types that use drillbuf to shore their value.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183777285
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarDecimalFunctions.java ---
@@ -0,0 +1,911 @@
+/*
+ * 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.fn.impl;
+
+import org.apache.drill.categories.SqlFunctionTest;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.test.BaseTestQuery;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.math.RoundingMode;
+
+@Category(SqlFunctionTest.class)
+public class TestVarDecimalFunctions extends BaseTestQuery {
+
+ // Tests for math functions
+
+ @Test
+ public void testDecimalAdd() throws Exception {
+ String query =
+ "select\n" +
+ // checks trimming of scale
+ "cast('999999999999999999999999999.92345678912' as DECIMAL(38, 11))\n" +
+ "+ cast('0.32345678912345678912345678912345678912' as DECIMAL(38, 38)) as s1,\n" +
+ // sanitary checks
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('123456789123456789123456789123456.789' as DECIMAL(36, 3)) as s2,\n" +
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('0' as DECIMAL(36, 3)) as s3,\n" +
+ "cast('15.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s4,\n" +
+ "cast('11.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s5,\n" +
+ "cast('0' as DECIMAL(36, 2)) - cast('12.93' as DECIMAL(36, 2)) as s6,\n" +
+ // check trimming (negative scale)
+ "cast('99999999999999999999999999992345678912' as DECIMAL(38, 0))\n" +
+ "+ cast('32345678912345678912345678912345678912' as DECIMAL(38, 0)) as s7";
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
+ testBuilder()
+ .sqlQuery(query)
+ .ordered()
+ .baselineColumns("s1", "s2", "s3", "s4", "s5", "s6", "s7")
+ .baselineValues(
+ new BigDecimal("999999999999999999999999999.92345678912")
+ .add(new BigDecimal("0.32345678912345678912345678912345678912"))
+ .round(new MathContext(38, RoundingMode.HALF_UP)),
+ new BigDecimal("1358024680358024680358024680358024.679"),
+ new BigDecimal("1234567891234567891234567891234567.890"),
+ new BigDecimal("2.09"), new BigDecimal("-1.91"), new BigDecimal("-12.93"),
+ new BigDecimal("1.3234567891234567891234567890469135782E+38"))
+ .build()
--- End diff --
Please replace with `go()`.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184028590
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java ---
@@ -668,46 +706,95 @@ public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
}
private static class DrillSameSqlReturnTypeInference implements SqlReturnTypeInference {
- private static final DrillSameSqlReturnTypeInference INSTANCE = new DrillSameSqlReturnTypeInference();
+ private static final DrillSameSqlReturnTypeInference INSTANCE = new DrillSameSqlReturnTypeInference(true);
--- End diff --
Thanks, renamed.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184027803
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java ---
@@ -169,12 +168,12 @@ private static void writeLiteral(RexLiteral literal, JsonOutput out) throws IOEx
return;
case DECIMAL:
+ // Still used double instead of decimal since the scale and precision of values are unknown
--- End diff --
Thanks, reworded.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183768790
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java ---
@@ -559,6 +560,19 @@ public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability)
if (matchNullability) {
return makeAbstractCast(type, exp);
}
+ // for the case when BigDecimal literal has a scale or precision
+ // that differs from the value from specified RelDataType, cast cannot be removed
+ // TODO: remove this code when CALCITE-1468 is fixed
+ if (type.getSqlTypeName() == SqlTypeName.DECIMAL && exp instanceof RexLiteral) {
--- End diff --
Please create the follow up Drill Jira.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183350828
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java ---
@@ -248,27 +227,61 @@ protected void readField(long recordsToReadInThisPass) {
}
}
- static class DictionaryDecimal18Reader extends FixedByteAlignedReader<Decimal18Vector> {
- DictionaryDecimal18Reader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
- ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, Decimal18Vector v,
- SchemaElement schemaElement) throws ExecutionSetupException {
+ static class DictionaryVarDecimalReader extends FixedByteAlignedReader<VarDecimalVector> {
+
+ DictionaryVarDecimalReader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
+ ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, VarDecimalVector v,
+ SchemaElement schemaElement) throws ExecutionSetupException {
super(parentReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, v, schemaElement);
}
// this method is called by its superclass during a read loop
@Override
protected void readField(long recordsToReadInThisPass) {
+ recordsReadInThisIteration =
+ Math.min(pageReader.currentPageCount - pageReader.valuesRead,
+ recordsToReadInThisPass - valuesReadInCurrentPass);
+
+ switch (columnDescriptor.getType()) {
+ case INT32:
+ if (usingDictionary) {
+ for (int i = 0; i < recordsReadInThisIteration; i++) {
+ byte[] bytes = Ints.toByteArray(pageReader.dictionaryValueReader.readInteger());
+ setValueBytes(i, bytes);
+ }
+ setWriteIndex();
+ } else {
+ super.readField(recordsToReadInThisPass);
+ }
+ break;
+ case INT64:
+ if (usingDictionary) {
+ for (int i = 0; i < recordsReadInThisIteration; i++) {
+ byte[] bytes = Longs.toByteArray(pageReader.dictionaryValueReader.readLong());
+ setValueBytes(i, bytes);
+ }
+ setWriteIndex();
+ } else {
+ super.readField(recordsToReadInThisPass);
+ }
+ break;
+ }
+ }
- recordsReadInThisIteration = Math.min(pageReader.currentPageCount
- - pageReader.valuesRead, recordsToReadInThisPass - valuesReadInCurrentPass);
+ /**
+ * Set the write Index. The next page that gets read might be a page that does not use dictionary encoding
+ * and we will go into the else condition below. The readField method of the parent class requires the
+ * writer index to be set correctly.
+ */
+ private void setWriteIndex() {
+ readLengthInBits = recordsReadInThisIteration * dataTypeLengthInBits;
+ readLength = (int) Math.ceil(readLengthInBits / 8.0);
--- End diff --
Do you know how this magic number was chosen?
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183762287
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/output/DecimalReturnTypeInference.java ---
@@ -281,20 +295,45 @@
@Override
public TypeProtos.MajorType getType(List<LogicalExpression> logicalExpressions, FunctionAttributes attributes) {
int scale = 0;
- int precision = 0;
// Get the max scale and precision from the inputs
for (LogicalExpression e : logicalExpressions) {
scale = Math.max(scale, e.getMajorType().getScale());
- precision = Math.max(precision, e.getMajorType().getPrecision());
}
- return (TypeProtos.MajorType.newBuilder()
- .setMinorType(attributes.getReturnValue().getType().getMinorType())
+ return TypeProtos.MajorType.newBuilder()
+ .setMinorType(TypeProtos.MinorType.VARDECIMAL)
.setScale(scale)
- .setPrecision(38)
- .setMode(TypeProtos.DataMode.REQUIRED)
- .build());
+ .setPrecision(DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision())
+ .setMode(TypeProtos.DataMode.OPTIONAL)
+ .build();
+ }
+ }
+
+ /**
+ * Return type calculation implementation for functions with return type set as
+ * {@link org.apache.drill.exec.expr.annotations.FunctionTemplate.ReturnType#DECIMAL_AVG_AGGREGATE}.
+ */
+ public static class DecimalAvgAggReturnTypeInference implements ReturnTypeInference {
--- End diff --
Please add information how precision and scale are calculated.
---
[GitHub] drill issue #1232: DRILL-6094: Decimal data type enhancements
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1232
I would recommend we wait till we have all testing complete, since this is a big feature.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184008595
--- Diff: exec/vector/src/main/codegen/templates/AbstractFieldReader.java ---
@@ -29,9 +29,9 @@
* This class is generated using freemarker and the ${.template_name} template.
*/
@SuppressWarnings("unused")
-abstract class AbstractFieldReader extends AbstractBaseReader implements FieldReader{
+abstract class AbstractFieldReader extends AbstractBaseReader implements FieldReader {
- AbstractFieldReader(){
+ AbstractFieldReader() {
--- End diff --
Done.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183765063
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/FrameSupportTemplate.java ---
@@ -300,7 +300,7 @@ public void cleanup() {
* @param index of row to aggregate
*/
public abstract void evaluatePeer(@Named("index") int index);
- public abstract void setupEvaluatePeer(@Named("incoming") VectorAccessible incoming, @Named("outgoing") VectorAccessible outgoing) throws SchemaChangeException;
+ public abstract void setupEvaluatePeer(@Named("incoming") WindowDataBatch incoming, @Named("outgoing") VectorAccessible outgoing) throws SchemaChangeException;
--- End diff --
Could you please explain this change?
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183356082
--- Diff: exec/vector/src/main/codegen/templates/ValueHolders.java ---
@@ -32,99 +32,81 @@
/*
* This class is generated using freemarker and the ${.template_name} template.
*/
-public final class ${className} implements ValueHolder{
+public final class ${className} implements ValueHolder {
public static final MajorType TYPE = Types.${mode.name?lower_case}(MinorType.${minor.class?upper_case});
- <#if mode.name == "Repeated">
+ <#if mode.name == "Repeated">
- /** The first index (inclusive) into the Vector. **/
- public int start;
+ /** The first index (inclusive) into the Vector. **/
+ public int start;
- /** The last index (exclusive) into the Vector. **/
- public int end;
+ /** The last index (exclusive) into the Vector. **/
+ public int end;
- /** The Vector holding the actual values. **/
- public ${minor.class}Vector vector;
+ /** The Vector holding the actual values. **/
+ public ${minor.class}Vector vector;
- <#else>
- public static final int WIDTH = ${type.width};
+ <#else>
+ public static final int WIDTH = ${type.width};
- <#if mode.name == "Optional">public int isSet;</#if>
- <#assign fields = minor.fields!type.fields />
- <#list fields as field>
- public ${field.type} ${field.name};
- </#list>
+ <#if mode.name == "Optional">public int isSet;</#if>
+ <#assign fields = minor.fields!type.fields />
+ <#list fields as field>
+ public ${field.type} ${field.name};
+ </#list>
- <#if minor.class.startsWith("Decimal")>
- public static final int maxPrecision = ${minor.maxPrecisionDigits};
- <#if minor.class.startsWith("Decimal28") || minor.class.startsWith("Decimal38")>
- public static final int nDecimalDigits = ${minor.nDecimalDigits};
+ <#if minor.class.startsWith("Decimal")>
+ public static final int maxPrecision = ${minor.maxPrecisionDigits};
+ <#if minor.class.startsWith("Decimal28") || minor.class.startsWith("Decimal38")>
--- End diff --
Please mark old decimal value holders as deprecated.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183354148
--- Diff: exec/vector/src/main/codegen/templates/AbstractPromotableFieldWriter.java ---
@@ -75,12 +75,19 @@ public void endList() {
<#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
<#assign fields = minor.fields!type.fields />
- <#if !minor.class?starts_with("Decimal") >
+ <#if minor.class?contains("VarDecimal")>
+ @Override
+ public void write${minor.class}(BigDecimal value) {
+ getWriter(MinorType.${name?upper_case}).write${minor.class}(value);
+ }
+ </#if>
+
@Override
public void write(${name}Holder holder) {
getWriter(MinorType.${name?upper_case}).write(holder);
}
+ <#if !minor.class?contains("Decimal") || minor.class?contains("VarDecimal")>
--- End diff --
Please add comment that this is done to cover previous decimal functionality.
---
[GitHub] drill issue #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/1232
LGTM.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183771627
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java ---
@@ -382,13 +407,26 @@ public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
final RelDataType operandType = opBinding.getOperandType(0);
final TypeProtos.MinorType inputMinorType = getDrillTypeFromCalciteType(operandType);
- if(TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.BIGINT))
+ if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.BIGINT))
== TypeProtos.MinorType.BIGINT) {
return createCalciteTypeWithNullability(
factory,
SqlTypeName.BIGINT,
isNullable);
- } else if(TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT8))
+ } else if (TypeCastRules.getLeastRestrictiveType(Lists.newArrayList(inputMinorType, TypeProtos.MinorType.FLOAT4))
--- End diff --
Please add explanation.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184035111
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarDecimalFunctions.java ---
@@ -0,0 +1,911 @@
+/*
+ * 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.fn.impl;
+
+import org.apache.drill.categories.SqlFunctionTest;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.test.BaseTestQuery;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.math.RoundingMode;
+
+@Category(SqlFunctionTest.class)
+public class TestVarDecimalFunctions extends BaseTestQuery {
+
+ // Tests for math functions
+
+ @Test
+ public void testDecimalAdd() throws Exception {
+ String query =
+ "select\n" +
+ // checks trimming of scale
+ "cast('999999999999999999999999999.92345678912' as DECIMAL(38, 11))\n" +
+ "+ cast('0.32345678912345678912345678912345678912' as DECIMAL(38, 38)) as s1,\n" +
+ // sanitary checks
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('123456789123456789123456789123456.789' as DECIMAL(36, 3)) as s2,\n" +
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('0' as DECIMAL(36, 3)) as s3,\n" +
+ "cast('15.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s4,\n" +
+ "cast('11.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s5,\n" +
+ "cast('0' as DECIMAL(36, 2)) - cast('12.93' as DECIMAL(36, 2)) as s6,\n" +
+ // check trimming (negative scale)
+ "cast('99999999999999999999999999992345678912' as DECIMAL(38, 0))\n" +
+ "+ cast('32345678912345678912345678912345678912' as DECIMAL(38, 0)) as s7";
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
+ testBuilder()
+ .sqlQuery(query)
+ .ordered()
+ .baselineColumns("s1", "s2", "s3", "s4", "s5", "s6", "s7")
+ .baselineValues(
+ new BigDecimal("999999999999999999999999999.92345678912")
+ .add(new BigDecimal("0.32345678912345678912345678912345678912"))
+ .round(new MathContext(38, RoundingMode.HALF_UP)),
+ new BigDecimal("1358024680358024680358024680358024.679"),
+ new BigDecimal("1234567891234567891234567891234567.890"),
+ new BigDecimal("2.09"), new BigDecimal("-1.91"), new BigDecimal("-12.93"),
+ new BigDecimal("1.3234567891234567891234567890469135782E+38"))
+ .build()
--- End diff --
Thanks, replaced.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184099659
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java ---
@@ -559,6 +560,19 @@ public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability)
if (matchNullability) {
return makeAbstractCast(type, exp);
}
+ // for the case when BigDecimal literal has a scale or precision
+ // that differs from the value from specified RelDataType, cast cannot be removed
+ // TODO: remove this code when CALCITE-1468 is fixed
+ if (type.getSqlTypeName() == SqlTypeName.DECIMAL && exp instanceof RexLiteral) {
--- End diff --
Created DRILL-6355.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183359169
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java ---
@@ -409,4 +409,30 @@ public void testNLJoinCorrectnessRightMultipleBatches() throws Exception {
setSessionOption(ExecConstants.SLICE_TARGET, 100000);
}
}
+
+ @Test
+ public void testNlJoinWithStringsInCondition() throws Exception {
+ try {
+ test(DISABLE_NLJ_SCALAR);
+ test(DISABLE_JOIN_OPTIMIZATION);
+
+ final String query =
--- End diff --
How these changes relate to decimals?
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183461299
--- Diff: exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java ---
@@ -20,12 +20,17 @@
import java.io.IOException;
import java.net.URL;
+import com.google.common.base.Function;
--- End diff --
How did you test backward compatibility?
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183349829
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java ---
@@ -228,14 +232,33 @@ private void newSchema() throws IOException {
setUp(schema, consumer);
}
- private PrimitiveType getPrimitiveType(MaterializedField field) {
+ protected PrimitiveType getPrimitiveType(MaterializedField field) {
MinorType minorType = field.getType().getMinorType();
String name = field.getName();
+ int length = ParquetTypeHelper.getLengthForMinorType(minorType);
PrimitiveTypeName primitiveTypeName = ParquetTypeHelper.getPrimitiveTypeNameForMinorType(minorType);
+ if (DecimalUtility.isDecimalType(minorType)) {
+ OptionManager optionManager = oContext.getFragmentContext().getOptions();
+ if (optionManager.getString(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS)
--- End diff --
It looks like using `writerOptions` is more preferred approach in this class.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183775219
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/stat/ParquetMetaStatCollector.java ---
@@ -152,6 +152,7 @@ private ColumnStatistics getStat(Object min, Object max, Long numNull,
}
if (min != null && max != null ) {
+ // TODO: add vardecimal type
--- End diff --
Please remove.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184002128
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/output/DecimalReturnTypeInference.java ---
@@ -281,20 +295,45 @@
@Override
public TypeProtos.MajorType getType(List<LogicalExpression> logicalExpressions, FunctionAttributes attributes) {
int scale = 0;
- int precision = 0;
// Get the max scale and precision from the inputs
for (LogicalExpression e : logicalExpressions) {
scale = Math.max(scale, e.getMajorType().getScale());
- precision = Math.max(precision, e.getMajorType().getPrecision());
}
- return (TypeProtos.MajorType.newBuilder()
- .setMinorType(attributes.getReturnValue().getType().getMinorType())
+ return TypeProtos.MajorType.newBuilder()
+ .setMinorType(TypeProtos.MinorType.VARDECIMAL)
.setScale(scale)
- .setPrecision(38)
- .setMode(TypeProtos.DataMode.REQUIRED)
- .build());
+ .setPrecision(DRILL_REL_DATATYPE_SYSTEM.getMaxNumericPrecision())
+ .setMode(TypeProtos.DataMode.OPTIONAL)
+ .build();
+ }
+ }
+
+ /**
+ * Return type calculation implementation for functions with return type set as
+ * {@link org.apache.drill.exec.expr.annotations.FunctionTemplate.ReturnType#DECIMAL_AVG_AGGREGATE}.
+ */
+ public static class DecimalAvgAggReturnTypeInference implements ReturnTypeInference {
--- End diff --
Thanks, added.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183354594
--- Diff: exec/vector/src/main/codegen/templates/NullReader.java ---
@@ -31,19 +31,19 @@
* This class is generated using freemarker and the ${.template_name} template.
*/
@SuppressWarnings("unused")
-public class NullReader extends AbstractBaseReader implements FieldReader{
+public class NullReader extends AbstractBaseReader implements FieldReader {
public static final NullReader INSTANCE = new NullReader();
public static final NullReader EMPTY_LIST_INSTANCE = new NullReader(Types.repeated(TypeProtos.MinorType.NULL));
public static final NullReader EMPTY_MAP_INSTANCE = new NullReader(Types.required(TypeProtos.MinorType.MAP));
private MajorType type;
- private NullReader(){
+ private NullReader() {
--- End diff --
Please remove `super()` here and below.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184051298
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestFixedlenDecimal.java ---
@@ -20,61 +20,74 @@
import org.apache.drill.categories.UnlikelyTest;
import org.apache.drill.test.BaseTestQuery;
import org.apache.drill.exec.planner.physical.PlannerSettings;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@Category({UnlikelyTest.class})
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 {
- testBuilder()
- .sqlQuery("select count(*) as c from %s where department_id is null", DATAFILE)
- .unOrdered()
- .baselineColumns("c")
- .baselineValues(1L)
- .build()
- .run();
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
+ testBuilder()
+ .sqlQuery("select count(*) as c from %s where department_id is null", DATAFILE)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(1L)
+ .build()
+ .run();
+ } finally {
+ resetSessionOption(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY);
+ }
}
@Test
public void testNotNullCount() throws Exception {
- testBuilder()
- .sqlQuery("select count(*) as c from %s where department_id is not null", DATAFILE)
- .unOrdered()
- .baselineColumns("c")
- .baselineValues(106L)
- .build()
- .run();
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
+ testBuilder()
+ .sqlQuery("select count(*) as c from %s where department_id is not null", DATAFILE)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(106L)
+ .build()
--- End diff --
Thanks, replaced here and in other places.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183352077
--- Diff: exec/vector/src/main/codegen/templates/AbstractFieldReader.java ---
@@ -29,9 +29,9 @@
* This class is generated using freemarker and the ${.template_name} template.
*/
@SuppressWarnings("unused")
-abstract class AbstractFieldReader extends AbstractBaseReader implements FieldReader{
+abstract class AbstractFieldReader extends AbstractBaseReader implements FieldReader {
- AbstractFieldReader(){
+ AbstractFieldReader() {
--- End diff --
You can remove `super()`.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184002812
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/stat/RangeExprEvaluator.java ---
@@ -219,6 +219,7 @@ private Statistics evalCastFunc(FunctionHolderExpression holderExpr, Statistics
return null; // cast func between srcType and destType is NOT allowed.
}
+ // TODO: add decimal support
--- End diff --
Thanks, removed.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183763504
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ---
@@ -409,7 +409,7 @@ public void clear() {
// Check if the field exists.
ValueVector v = fieldVectorMap.get(field.getName());
- if (v == null || v.getClass() != clazz) {
+ if (v == null || !v.getField().getType().equals(field.getType())) {
--- End diff --
Please add explanation comment.
---
[GitHub] drill issue #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1232
@arina-ielchiieva, @parthchandra could you please take a look at this?
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184009513
--- Diff: exec/vector/src/main/codegen/templates/ValueHolders.java ---
@@ -32,99 +32,81 @@
/*
* This class is generated using freemarker and the ${.template_name} template.
*/
-public final class ${className} implements ValueHolder{
+public final class ${className} implements ValueHolder {
public static final MajorType TYPE = Types.${mode.name?lower_case}(MinorType.${minor.class?upper_case});
- <#if mode.name == "Repeated">
+ <#if mode.name == "Repeated">
- /** The first index (inclusive) into the Vector. **/
- public int start;
+ /** The first index (inclusive) into the Vector. **/
+ public int start;
- /** The last index (exclusive) into the Vector. **/
- public int end;
+ /** The last index (exclusive) into the Vector. **/
+ public int end;
- /** The Vector holding the actual values. **/
- public ${minor.class}Vector vector;
+ /** The Vector holding the actual values. **/
+ public ${minor.class}Vector vector;
- <#else>
- public static final int WIDTH = ${type.width};
+ <#else>
+ public static final int WIDTH = ${type.width};
- <#if mode.name == "Optional">public int isSet;</#if>
- <#assign fields = minor.fields!type.fields />
- <#list fields as field>
- public ${field.type} ${field.name};
- </#list>
+ <#if mode.name == "Optional">public int isSet;</#if>
+ <#assign fields = minor.fields!type.fields />
+ <#list fields as field>
+ public ${field.type} ${field.name};
+ </#list>
- <#if minor.class.startsWith("Decimal")>
- public static final int maxPrecision = ${minor.maxPrecisionDigits};
- <#if minor.class.startsWith("Decimal28") || minor.class.startsWith("Decimal38")>
- public static final int nDecimalDigits = ${minor.nDecimalDigits};
+ <#if minor.class.startsWith("Decimal")>
+ public static final int maxPrecision = ${minor.maxPrecisionDigits};
+ <#if minor.class.startsWith("Decimal28") || minor.class.startsWith("Decimal38")>
--- End diff --
Thanks, good idea, marked as deprecated and added a comment to use VarDecimalHolder instead.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184010194
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestVarlenDecimal.java ---
@@ -0,0 +1,153 @@
+/*
+ * 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.commons.io.FileUtils;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.hamcrest.CoreMatchers;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.math.BigDecimal;
+import java.nio.file.Paths;
+
+public class TestVarlenDecimal extends BaseTestQuery {
+
+ private static final String DATAFILE = "cp.`parquet/varlenDecimal.parquet`";
+
+ @Test
+ public void testNullCount() throws Exception {
+ String query = String.format("select count(*) as c from %s where department_id is null", DATAFILE);
--- End diff --
Agree, removed `String.format` here and in other places.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184035055
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarDecimalFunctions.java ---
@@ -0,0 +1,911 @@
+/*
+ * 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.fn.impl;
+
+import org.apache.drill.categories.SqlFunctionTest;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.test.BaseTestQuery;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.math.RoundingMode;
+
+@Category(SqlFunctionTest.class)
+public class TestVarDecimalFunctions extends BaseTestQuery {
+
+ // Tests for math functions
+
+ @Test
+ public void testDecimalAdd() throws Exception {
+ String query =
+ "select\n" +
+ // checks trimming of scale
+ "cast('999999999999999999999999999.92345678912' as DECIMAL(38, 11))\n" +
+ "+ cast('0.32345678912345678912345678912345678912' as DECIMAL(38, 38)) as s1,\n" +
+ // sanitary checks
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('123456789123456789123456789123456.789' as DECIMAL(36, 3)) as s2,\n" +
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('0' as DECIMAL(36, 3)) as s3,\n" +
+ "cast('15.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s4,\n" +
+ "cast('11.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s5,\n" +
+ "cast('0' as DECIMAL(36, 2)) - cast('12.93' as DECIMAL(36, 2)) as s6,\n" +
+ // check trimming (negative scale)
+ "cast('99999999999999999999999999992345678912' as DECIMAL(38, 0))\n" +
+ "+ cast('32345678912345678912345678912345678912' as DECIMAL(38, 0)) as s7";
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
--- End diff --
Thanks, used `@BeforeClass` and `@AfterClass`.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183770874
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/TypeInferenceUtils.java ---
@@ -668,46 +706,95 @@ public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
}
private static class DrillSameSqlReturnTypeInference implements SqlReturnTypeInference {
- private static final DrillSameSqlReturnTypeInference INSTANCE = new DrillSameSqlReturnTypeInference();
+ private static final DrillSameSqlReturnTypeInference INSTANCE = new DrillSameSqlReturnTypeInference(true);
--- End diff --
Please rename `INSTANCE`.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183998921
--- Diff: exec/java-exec/src/main/codegen/templates/Decimal/CastDecimalFloat.java ---
@@ -86,24 +46,21 @@ public void eval() {
*/
@SuppressWarnings("unused")
-@FunctionTemplate(name = "cast${type.to?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.NULL_IF_NULL)
+@FunctionTemplate(name = "cast${type.to?upper_case}",
+ scope = FunctionTemplate.FunctionScope.SIMPLE,
+ nulls=NullHandling.NULL_IF_NULL)
--- End diff --
Thanks, added spaces.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183778375
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestFixedlenDecimal.java ---
@@ -20,61 +20,74 @@
import org.apache.drill.categories.UnlikelyTest;
import org.apache.drill.test.BaseTestQuery;
import org.apache.drill.exec.planner.physical.PlannerSettings;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@Category({UnlikelyTest.class})
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 {
- testBuilder()
- .sqlQuery("select count(*) as c from %s where department_id is null", DATAFILE)
- .unOrdered()
- .baselineColumns("c")
- .baselineValues(1L)
- .build()
- .run();
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
+ testBuilder()
+ .sqlQuery("select count(*) as c from %s where department_id is null", DATAFILE)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(1L)
+ .build()
+ .run();
+ } finally {
+ resetSessionOption(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY);
+ }
}
@Test
public void testNotNullCount() throws Exception {
- testBuilder()
- .sqlQuery("select count(*) as c from %s where department_id is not null", DATAFILE)
- .unOrdered()
- .baselineColumns("c")
- .baselineValues(106L)
- .build()
- .run();
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
+ testBuilder()
+ .sqlQuery("select count(*) as c from %s where department_id is not null", DATAFILE)
+ .unOrdered()
+ .baselineColumns("c")
+ .baselineValues(106L)
+ .build()
--- End diff --
Please consider `go()`. Please check in other classes as well.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183749830
--- Diff: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcRecordReader.java ---
@@ -225,10 +247,10 @@ public int next() {
int counter = 0;
Boolean b = true;
try {
- while (counter < 4095 && b == true) { // loop at 4095 since nullables use one more than record count and we
+ while (counter < 4095 && b) { // loop at 4095 since nullables use one more than record count and we
// allocate on powers of two.
b = resultSet.next();
- if(b == false) {
+ if(!b) {
--- End diff --
Please add space.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184062425
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java ---
@@ -248,27 +227,61 @@ protected void readField(long recordsToReadInThisPass) {
}
}
- static class DictionaryDecimal18Reader extends FixedByteAlignedReader<Decimal18Vector> {
- DictionaryDecimal18Reader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
- ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, Decimal18Vector v,
- SchemaElement schemaElement) throws ExecutionSetupException {
+ static class DictionaryVarDecimalReader extends FixedByteAlignedReader<VarDecimalVector> {
+
+ DictionaryVarDecimalReader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
+ ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, VarDecimalVector v,
+ SchemaElement schemaElement) throws ExecutionSetupException {
super(parentReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, v, schemaElement);
}
// this method is called by its superclass during a read loop
@Override
protected void readField(long recordsToReadInThisPass) {
+ recordsReadInThisIteration =
+ Math.min(pageReader.currentPageCount - pageReader.valuesRead,
+ recordsToReadInThisPass - valuesReadInCurrentPass);
+
+ switch (columnDescriptor.getType()) {
+ case INT32:
+ if (usingDictionary) {
+ for (int i = 0; i < recordsReadInThisIteration; i++) {
+ byte[] bytes = Ints.toByteArray(pageReader.dictionaryValueReader.readInteger());
+ setValueBytes(i, bytes);
+ }
+ setWriteIndex();
+ } else {
+ super.readField(recordsToReadInThisPass);
+ }
+ break;
+ case INT64:
+ if (usingDictionary) {
+ for (int i = 0; i < recordsReadInThisIteration; i++) {
+ byte[] bytes = Longs.toByteArray(pageReader.dictionaryValueReader.readLong());
+ setValueBytes(i, bytes);
+ }
+ setWriteIndex();
+ } else {
+ super.readField(recordsToReadInThisPass);
+ }
+ break;
+ }
+ }
- recordsReadInThisIteration = Math.min(pageReader.currentPageCount
- - pageReader.valuesRead, recordsToReadInThisPass - valuesReadInCurrentPass);
+ /**
+ * Set the write Index. The next page that gets read might be a page that does not use dictionary encoding
+ * and we will go into the else condition below. The readField method of the parent class requires the
+ * writer index to be set correctly.
+ */
+ private void setWriteIndex() {
+ readLengthInBits = recordsReadInThisIteration * dataTypeLengthInBits;
+ readLength = (int) Math.ceil(readLengthInBits / 8.0);
--- End diff --
This is the number of bits in a byte, but a double value is used to avoid integer division. Thanks for pointing this, replaced by constant.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183777825
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestVarDecimalFunctions.java ---
@@ -0,0 +1,911 @@
+/*
+ * 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.fn.impl;
+
+import org.apache.drill.categories.SqlFunctionTest;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.test.BaseTestQuery;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.math.RoundingMode;
+
+@Category(SqlFunctionTest.class)
+public class TestVarDecimalFunctions extends BaseTestQuery {
+
+ // Tests for math functions
+
+ @Test
+ public void testDecimalAdd() throws Exception {
+ String query =
+ "select\n" +
+ // checks trimming of scale
+ "cast('999999999999999999999999999.92345678912' as DECIMAL(38, 11))\n" +
+ "+ cast('0.32345678912345678912345678912345678912' as DECIMAL(38, 38)) as s1,\n" +
+ // sanitary checks
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('123456789123456789123456789123456.789' as DECIMAL(36, 3)) as s2,\n" +
+ "cast('1234567891234567891234567891234567.89' as DECIMAL(36, 2))\n" +
+ "+ cast('0' as DECIMAL(36, 3)) as s3,\n" +
+ "cast('15.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s4,\n" +
+ "cast('11.02' as DECIMAL(4, 2)) - cast('12.93' as DECIMAL(4, 2)) as s5,\n" +
+ "cast('0' as DECIMAL(36, 2)) - cast('12.93' as DECIMAL(36, 2)) as s6,\n" +
+ // check trimming (negative scale)
+ "cast('99999999999999999999999999992345678912' as DECIMAL(38, 0))\n" +
+ "+ cast('32345678912345678912345678912345678912' as DECIMAL(38, 0)) as s7";
+ try {
+ alterSession(PlannerSettings.ENABLE_DECIMAL_DATA_TYPE_KEY, true);
--- End diff --
Consider consider using `@BeforeClass` and `@AfterClass` for decimal option.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184007677
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Hash64WithSeedAsDouble.java ---
@@ -265,6 +268,42 @@ public void eval() {
}
}
+ @FunctionTemplate(name = "hash64AsDouble", scope = FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+ public static class VarDecimalHash implements DrillSimpleFunc {
+ @Param VarDecimalHolder in;
+ @Param BigIntHolder seed;
+ @Output BigIntHolder out;
+
+ public void setup() {
+ }
+
+ public void eval() {
+ java.math.BigDecimal input = org.apache.drill.exec.util.DecimalUtility.getBigDecimalFromDrillBuf(in.buffer,
+ in.start, in.end - in.start, in.scale);
+ out.value = org.apache.drill.exec.expr.fn.impl.HashHelper.hash64(input.doubleValue(), seed.value);
+ }
+ }
+
+ @FunctionTemplate(name = "hash64AsDouble", scope = FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL)
+ public static class NullableVarDecimalHash implements DrillSimpleFunc {
+ @Param NullableVarDecimalHolder in;
+ @Param BigIntHolder seed;
+ @Output BigIntHolder out;
+
+ public void setup() {
+ }
+
+ public void eval() {
+ if (in.isSet == 0) {
+ out.value = seed.value;
+ } else {
+ java.math.BigDecimal input = org.apache.drill.exec.util.DecimalUtility.getBigDecimalFromDrillBuf(in.buffer,
+ in.start, in.end - in.start, in.scale);
+ out.value = org.apache.drill.exec.expr.fn.impl.HashHelper.hash64(input.doubleValue(), seed.value);
+ }
+ }
+ }
--- End diff --
Thanks for pointing this, removed functions that use old decimals.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r183766242
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java ---
@@ -169,12 +168,12 @@ private static void writeLiteral(RexLiteral literal, JsonOutput out) throws IOEx
return;
case DECIMAL:
+ // Still used double instead of decimal since the scale and precision of values are unknown
--- End diff --
Please adjust comment.
---
[GitHub] drill pull request #1232: DRILL-6094: Decimal data type enhancements
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1232#discussion_r184065631
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/FrameSupportTemplate.java ---
@@ -300,7 +300,7 @@ public void cleanup() {
* @param index of row to aggregate
*/
public abstract void evaluatePeer(@Named("index") int index);
- public abstract void setupEvaluatePeer(@Named("incoming") VectorAccessible incoming, @Named("outgoing") VectorAccessible outgoing) throws SchemaChangeException;
+ public abstract void setupEvaluatePeer(@Named("incoming") WindowDataBatch incoming, @Named("outgoing") VectorAccessible outgoing) throws SchemaChangeException;
--- End diff --
On the earlier stage of making changes, compilation error has appeared for runtime-generated code without this change, but for now, I don't see it without this change, so I reverted it.
---