[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16525711#comment-16525711 ] Bridget Bevens commented on DRILL-6094: --- I edited the following pages based on the work done for DECIMAL data type and input from Volodymyr. (Thank you, [~vvysotskyi]!) https://drill.apache.org/docs/aggregate-and-aggregate-statistical/ https://drill.apache.org/docs/aggregate-window-functions/ https://drill.apache.org/docs/data-type-conversion/#data-type-conversion-examples https://drill.apache.org/docs/supported-data-types/ https://drill.apache.org/docs/parquet-format/ https://drill.apache.org/docs/math-and-trig/ I also created a new section called “Decimal Data Type” on this page to cover some of the changes: https://drill.apache.org/docs/supported-data-types/#decimal-data-type I'm setting doc status to doc-complete, but please let me know if I missed anything or need to make any changes. Thanks, Bridget > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16454912#comment-16454912 ] ASF GitHub Bot commented on DRILL-6094: --- Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/1232 +1 for the C++ part. Looks really good. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16454748#comment-16454748 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16453824#comment-16453824 ] ASF GitHub Bot commented on DRILL-6094: --- Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1232 LGTM. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452483#comment-16452483 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452488#comment-16452488 ] ASF GitHub Bot commented on DRILL-6094: --- 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 { -DictionaryDecimal18Reader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor, - ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, Decimal18Vector v, - SchemaElement schemaElement) throws ExecutionSetupException { + static class DictionaryVarDecimalReader extends FixedByteAlignedReader { + +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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452476#comment-16452476 ] ASF GitHub Bot commented on DRILL-6094: --- 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; -<#assign fields = minor.fields!type.fields /> -<#list fields as field> -public ${field.type} ${field.name}; - + <#if mode.name == "Optional">public int isSet; + <#assign fields = minor.fields!type.fields /> + <#list fields as field> + public ${field.type} ${field.name}; + -<#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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452478#comment-16452478 ] ASF GitHub Bot commented on DRILL-6094: --- 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, 10); } } + + @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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452473#comment-16452473 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452466#comment-16452466 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452484#comment-16452484 ] ASF GitHub Bot commented on DRILL-6094: --- 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('999.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('2345678912' 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("999.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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452482#comment-16452482 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452475#comment-16452475 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452465#comment-16452465 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452487#comment-16452487 ] ASF GitHub Bot commented on DRILL-6094: --- 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 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452486#comment-16452486 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452485#comment-16452485 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452469#comment-16452469 ] ASF GitHub Bot commented on DRILL-6094: --- 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); + } + + @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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452468#comment-16452468 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452467#comment-16452467 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452474#comment-16452474 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452477#comment-16452477 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452470#comment-16452470 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452480#comment-16452480 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452471#comment-16452471 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452479#comment-16452479 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452481#comment-16452481 ] ASF GitHub Bot commented on DRILL-6094: --- 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('999.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('2345678912' 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`. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452472#comment-16452472 ] ASF GitHub Bot commented on DRILL-6094: --- 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`. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450088#comment-16450088 ] ASF GitHub Bot commented on DRILL-6094: --- 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; -<#assign fields = minor.fields!type.fields /> -<#list fields as field> -public ${field.type} ${field.name}; - + <#if mode.name == "Optional">public int isSet; + <#assign fields = minor.fields!type.fields /> + <#list fields as field> + public ${field.type} ${field.name}; + -<#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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450080#comment-16450080 ] ASF GitHub Bot commented on DRILL-6094: --- 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` > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450098#comment-16450098 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450093#comment-16450093 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450099#comment-16450099 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450086#comment-16450086 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450092#comment-16450092 ] ASF GitHub Bot commented on DRILL-6094: --- 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()`. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450085#comment-16450085 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450094#comment-16450094 ] ASF GitHub Bot commented on DRILL-6094: --- 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); + } + + @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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450090#comment-16450090 ] ASF GitHub Bot commented on DRILL-6094: --- 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('999.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('2345678912' 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("999.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()`. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450091#comment-16450091 ] ASF GitHub Bot commented on DRILL-6094: --- 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 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450081#comment-16450081 ] ASF GitHub Bot commented on DRILL-6094: --- 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? > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450101#comment-16450101 ] ASF GitHub Bot commented on DRILL-6094: --- 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`. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450097#comment-16450097 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450079#comment-16450079 ] ASF GitHub Bot commented on DRILL-6094: --- 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 { -DictionaryDecimal18Reader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor, - ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, Decimal18Vector v, - SchemaElement schemaElement) throws ExecutionSetupException { + static class DictionaryVarDecimalReader extends FixedByteAlignedReader { + +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? > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450087#comment-16450087 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450082#comment-16450082 ] ASF GitHub Bot commented on DRILL-6094: --- 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, 10); } } + + @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? > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450083#comment-16450083 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450095#comment-16450095 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450089#comment-16450089 ] ASF GitHub Bot commented on DRILL-6094: --- 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? > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450096#comment-16450096 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450078#comment-16450078 ] ASF GitHub Bot commented on DRILL-6094: --- 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... > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450084#comment-16450084 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450100#comment-16450100 ] ASF GitHub Bot commented on DRILL-6094: --- 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('999.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('2345678912' 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16450077#comment-16450077 ] ASF GitHub Bot commented on DRILL-6094: --- 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. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Labels: doc-impacting > Fix For: 1.14.0 > > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16445823#comment-16445823 ] ASF GitHub Bot commented on DRILL-6094: --- 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 Date: 2016-02-09T22:37:47Z DRILL-4184: Support variable length decimal fields in parquet commit 84a31b92dc86873d14a7be601d189bb1cbd59b7d Author: Volodymyr Vysotskyi Date: 2018-04-05T12:35:42Z DRILL-6094: Decimal data type enhancements Add ExprVisitors for VARDECIMAL commit e93cd93211ffa84586ee0d21dab26b128d81b290 Author: Volodymyr Vysotskyi 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 Date: 2018-04-03T13:27:25Z DRILL-6094: Refresh protobuf C++ source files commit fbe93c46ed8a7e85a95da3ae145bfc00b7f17dd0 Author: Volodymyr Vysotskyi 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 Date: 2018-04-05T14:48:31Z DRILL-6094: Enable DECIMAL data type by default commit 46ce3d1504618ae7e547479280f5d60b35ec1caa Author: Volodymyr Vysotskyi Date: 2018-04-05T14:51:39Z DRILL-6094: Add unit tests for DECIMAL data type commit f62545390a37425b9fbb21881e4cd12ade337e73 Author: Volodymyr Vysotskyi Date: 2018-04-05T14:58:36Z DRILL-6094: Changes in C++ files commit 7a04309605c0cd869a9eb55450aaa43c0088b425 Author: Volodymyr Vysotskyi Date: 2018-04-10T12:55:17Z DRILL-6094: Fix mapping for NLJ when literal with non-primitive type is used in join conditions > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16445834#comment-16445834 ] ASF GitHub Bot commented on DRILL-6094: --- 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? > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (DRILL-6094) Decimal data type enhancements
[ https://issues.apache.org/jira/browse/DRILL-6094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435914#comment-16435914 ] Vitalii Diravka commented on DRILL-6094: Looks like ARROW-61 is not an issue for Drill anymore. Drill 1.6.0 version: {code:java} SELECT CAST('922337203685477587' as DECIMAL(18, 0)) > CAST('0.1' as DECIMAL(18, 1)) IS_FIRST_GREATER FROM (VALUES(1)) -- | IS_FIRST_GREATER| -- | false | -- select _DECIMAL_decimal18 > _DECIMAL_decimal18_2 GREATER_THAN, _DECIMAL_decimal18, _DECIMAL_decimal18_2 from dfs.`/tmp/parquet/drill/parquet_test_file_simple` t | GREATER_THAN| _DECIMAL_decimal18 CAST('0.1' as DECIMAL(18, 1)) IS_FIRST_GREATER FROM (VALUES(1)) IS_FIRST_GREATER true select _DECIMAL_decimal18 > _DECIMAL_decimal18_2 IS_FIRST_GREATER, _DECIMAL_decimal18, _DECIMAL_decimal18_2" + " from dfs.root.`/tmp/parquet/drill/parquet_test_file_simple` t IS_FIRST_GREATER,_DECIMAL_decimal18,_DECIMAL_decimal18_2 true,922337203685477587,0.1 {code} For the first query Calcite simplifies the predicaste in the project to true and for the second query casting to FLOAT8 is used now (and FLOAT8 values are compared). Therefore _adjustScaleMultiply_ is still buggy and isn't needed anymore (after moving to VARDECIMAL) I recommend to remove it from the project. > Decimal data type enhancements > -- > > Key: DRILL-6094 > URL: https://issues.apache.org/jira/browse/DRILL-6094 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.12.0 >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > > Currently, Decimal types are disabled by default since existing Decimal > implementation has a lot of flaws and performance problems. The goal of this > Jira to describe majority of them and possible ways of improving existing > implementation to be able to enable Decimal data types by default. -- This message was sent by Atlassian JIRA (v7.6.3#76005)