This is an automated email from the ASF dual-hosted git repository. arina pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push: new 3d2ee04 DRILL-6797: Fix UntypedNull handling for complex types 3d2ee04 is described below commit 3d2ee04fad7800bdec8bb71665548344f3cd27ae Author: Arina Ielchiieva <arina.yelchiy...@gmail.com> AuthorDate: Thu Oct 11 19:25:17 2018 +0300 DRILL-6797: Fix UntypedNull handling for complex types --- .../codegen/templates/EventBasedRecordWriter.java | 3 + .../apache/drill/exec/expr/EvaluationVisitor.java | 2 +- .../exec/expr/fn/impl/CompareUntypedNull.java | 73 ++++++++++++++ .../exec/store/parquet/ParquetRecordWriter.java | 7 +- .../java/org/apache/drill/TestUntypedNull.java | 110 +++++++++++++++++++++ .../codegen/templates/AbstractFieldReader.java | 4 + .../main/codegen/templates/BasicTypeHelper.java | 3 + .../drill/exec/vector/UntypedNullHolder.java | 2 +- .../drill/exec/vector/UntypedNullVector.java | 5 +- .../vector/complex/impl/AbstractBaseReader.java | 11 +++ .../UntypedHolderReaderImpl.java} | 38 +++++-- .../FieldReader.java => impl/UntypedReader.java} | 17 ++-- .../UntypedReaderImpl.java} | 37 +++++-- .../exec/vector/complex/reader/FieldReader.java | 3 +- 14 files changed, 286 insertions(+), 29 deletions(-) diff --git a/exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java b/exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java index 8c8097d..7357243 100644 --- a/exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java +++ b/exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java @@ -136,6 +136,9 @@ public class EventBasedRecordWriter { case LIST: return recordWriter.getNewRepeatedListConverter(fieldId, fieldName, reader); + case NULL: + return recordWriter.getNewNullableIntConverter(fieldId, fieldName, reader); + <#list vv.types as type> <#list type.minor as minor> case ${minor.class?upper_case}: diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java index e0f34e8..8685130 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java @@ -344,7 +344,7 @@ public class EvaluationVisitor { } else if (e instanceof HoldingContainerExpression) { return ((HoldingContainerExpression) e).getContainer(); } else if (e instanceof NullExpression) { - return generator.declare(Types.optional(MinorType.INT)); + return generator.declare(e.getMajorType()); } else if (e instanceof TypedNullConstant) { return generator.declare(e.getMajorType()); } else { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CompareUntypedNull.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CompareUntypedNull.java new file mode 100644 index 0000000..fa2765c --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CompareUntypedNull.java @@ -0,0 +1,73 @@ +/* + * 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.expr.fn.impl; + +import org.apache.drill.exec.expr.DrillSimpleFunc; +import org.apache.drill.exec.expr.annotations.FunctionTemplate; +import org.apache.drill.exec.expr.annotations.Output; +import org.apache.drill.exec.expr.annotations.Param; +import org.apache.drill.exec.expr.fn.FunctionGenerationHelper; +import org.apache.drill.exec.expr.holders.IntHolder; +import org.apache.drill.exec.vector.UntypedNullHolder; + +public class CompareUntypedNull { + + @FunctionTemplate(name = FunctionGenerationHelper.COMPARE_TO_NULLS_HIGH, + scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL) + public static class CompareUntypedNullHigh implements DrillSimpleFunc { + + @Param + UntypedNullHolder left; + + @Param + UntypedNullHolder right; + + @Output + IntHolder out; + + public void setup() { + } + + public void eval() { + out.value = 0; + } + } + + @FunctionTemplate(name = FunctionGenerationHelper.COMPARE_TO_NULLS_LOW, + scope = FunctionTemplate.FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.INTERNAL) + public static class CompareUntypedNullLow implements DrillSimpleFunc { + + @Param + UntypedNullHolder left; + + @Param + UntypedNullHolder right; + + @Output + IntHolder out; + + public void setup() { + } + + public void eval() { + out.value = 0; + } + } + +} + diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java index 6affae1..45233c4 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java @@ -28,6 +28,7 @@ import java.util.Map; import org.apache.drill.common.exceptions.DrillRuntimeException; import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.types.TypeProtos; import org.apache.drill.common.types.TypeProtos.DataMode; import org.apache.drill.common.types.TypeProtos.MinorType; import org.apache.drill.common.types.Types; @@ -284,7 +285,7 @@ public class ParquetRecordWriter extends ParquetOutputRecordWriter { private Type getType(MaterializedField field) { MinorType minorType = field.getType().getMinorType(); DataMode dataMode = field.getType().getMode(); - switch(minorType) { + switch (minorType) { case MAP: List<Type> types = Lists.newArrayList(); for (MaterializedField childField : field.getChildren()) { @@ -293,6 +294,10 @@ public class ParquetRecordWriter extends ParquetOutputRecordWriter { return new GroupType(dataMode == DataMode.REPEATED ? Repetition.REPEATED : Repetition.OPTIONAL, field.getName(), types); case LIST: throw new UnsupportedOperationException("Unsupported type " + minorType); + case NULL: + MaterializedField newField = field.withType( + TypeProtos.MajorType.newBuilder().setMinorType(MinorType.INT).setMode(DataMode.OPTIONAL).build()); + return getPrimitiveType(newField); default: return getPrimitiveType(field); } diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestUntypedNull.java b/exec/java-exec/src/test/java/org/apache/drill/TestUntypedNull.java new file mode 100644 index 0000000..4976947 --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/TestUntypedNull.java @@ -0,0 +1,110 @@ +/* + * 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; + +import org.apache.drill.categories.SqlFunctionTest; +import org.apache.drill.exec.ExecConstants; +import org.apache.drill.test.ClusterFixture; +import org.apache.drill.test.ClusterFixtureBuilder; +import org.apache.drill.test.ClusterTest; +import org.apache.drill.test.QueryBuilder; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +@Category(SqlFunctionTest.class) +public class TestUntypedNull extends ClusterTest { + + @BeforeClass + public static void setup() throws Exception { + ClusterFixtureBuilder builder = ClusterFixture.builder(dirTestWatcher); + startCluster(builder); + } + + @Test + public void testSplitFunction() throws Exception { + String query = "select split(n_name, ' ') [1] from cp.`tpch/nation.parquet`\n" + + "where n_nationkey = -1 group by n_name order by n_name limit 10"; + QueryBuilder.QuerySummary summary = queryBuilder().sql(query).run(); + assertTrue(summary.succeeded()); + assertEquals(0, summary.recordCount()); + } + + @Test + public void testWindowFunction() throws Exception { + String query = "select row_number() over (partition by split(n_name, ' ') [1])\n" + + "from cp.`tpch/nation.parquet` where n_nationkey = -1"; + QueryBuilder.QuerySummary summary = queryBuilder().sql(query).run(); + assertTrue(summary.succeeded()); + assertEquals(0, summary.recordCount()); + } + + @Test + public void testUnion() throws Exception { + String query = "select split(n_name, ' ') [1] from cp.`tpch/nation.parquet` where n_nationkey = -1 group by n_name\n" + + "union\n" + + "select split(n_name, ' ') [1] from cp.`tpch/nation.parquet` where n_nationkey = -1 group by n_name"; + QueryBuilder.QuerySummary summary = queryBuilder().sql(query).run(); + assertTrue(summary.succeeded()); + assertEquals(0, summary.recordCount()); + } + + @Test + public void testTableCreation() throws Exception { + String tablePrefix = "table_"; + List<String> formats = Arrays.asList("parquet", "json", "csv"); + try { + for (String format : formats) { + client.alterSession(ExecConstants.OUTPUT_FORMAT_OPTION, format); + String query = String.format("create table dfs.tmp.%s%s as\n" + + "select split(n_name, ' ') [1] from cp.`tpch/nation.parquet` where n_nationkey = -1 group by n_name", + tablePrefix, format); + QueryBuilder.QuerySummary summary = queryBuilder().sql(query).run(); + assertTrue(summary.succeeded()); + assertEquals(1, summary.recordCount()); + } + } finally { + client.resetSession(ExecConstants.OUTPUT_FORMAT_OPTION); + for (String format : formats) { + queryBuilder().sql(String.format("drop table if exists dfs.tmp.%s%s", tablePrefix, format)).run(); + } + } + } + + @Test + public void testTypeAndMode() throws Exception { + String query = "select\n" + + "typeof(split(n_name, ' ') [1]),\n" + + "drilltypeof(split(n_name, ' ') [1]),\n" + + "sqltypeof(split(n_name, ' ') [1]),\n" + + "modeof(split(n_name, ' ') [1])\n" + + "from cp.`tpch/nation.parquet`\n" + + "where n_nationkey = -1"; + QueryBuilder.QuerySummary summary = queryBuilder().sql(query).run(); + assertTrue(summary.succeeded()); + assertEquals(0, summary.recordCount()); + } + +} + diff --git a/exec/vector/src/main/codegen/templates/AbstractFieldReader.java b/exec/vector/src/main/codegen/templates/AbstractFieldReader.java index 4e5856b..4a763c2 100644 --- a/exec/vector/src/main/codegen/templates/AbstractFieldReader.java +++ b/exec/vector/src/main/codegen/templates/AbstractFieldReader.java @@ -103,6 +103,10 @@ abstract class AbstractFieldReader extends AbstractBaseReader implements FieldRe } </#list></#list> + public void read(int arrayIndex, UntypedNullHolder holder) { + fail("UntypedNullHolder"); + } + public FieldReader reader(String name) { fail("reader(String name)"); return null; diff --git a/exec/vector/src/main/codegen/templates/BasicTypeHelper.java b/exec/vector/src/main/codegen/templates/BasicTypeHelper.java index 02e1c27..430a41b 100644 --- a/exec/vector/src/main/codegen/templates/BasicTypeHelper.java +++ b/exec/vector/src/main/codegen/templates/BasicTypeHelper.java @@ -17,6 +17,7 @@ */ import org.apache.drill.exec.vector.UntypedNullHolder; import org.apache.drill.exec.vector.UntypedNullVector; +import org.apache.drill.exec.vector.complex.impl.UntypedHolderReaderImpl; <@pp.dropOutputFile /> <@pp.changeOutputFile name="/org/apache/drill/exec/expr/BasicTypeHelper.java" /> @@ -221,6 +222,8 @@ public class BasicTypeHelper { } </#list> </#list> + case NULL: + return UntypedHolderReaderImpl.class; default: break; } diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullHolder.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullHolder.java index 90356c1..b65f866 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullHolder.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullHolder.java @@ -24,7 +24,7 @@ import org.apache.drill.exec.expr.holders.ValueHolder; public class UntypedNullHolder implements ValueHolder { public static final TypeProtos.MajorType TYPE = Types.optional(TypeProtos.MinorType.NULL); public static final int WIDTH = 0; - public int isSet = 1; + public int isSet = 0; public TypeProtos.MajorType getType() {return TYPE;} diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java index f7536d8..9dd8480 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/UntypedNullVector.java @@ -18,6 +18,7 @@ package org.apache.drill.exec.vector; +import org.apache.drill.exec.vector.complex.impl.UntypedReaderImpl; import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; import io.netty.buffer.DrillBuf; import org.apache.drill.exec.memory.BufferAllocator; @@ -50,7 +51,9 @@ public final class UntypedNullVector extends BaseDataValueVector implements Fixe } @Override - public FieldReader getReader() { throw new UnsupportedOperationException(); } + public FieldReader getReader() { + return new UntypedReaderImpl(); + } @Override public int getBufferSizeFor(final int valueCount) { return 0; } diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java index 7c46f0b..668a332 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/AbstractBaseReader.java @@ -23,6 +23,7 @@ import org.apache.drill.common.types.TypeProtos.MajorType; import org.apache.drill.common.types.Types; import org.apache.drill.exec.expr.holders.UnionHolder; import org.apache.drill.exec.record.MaterializedField; +import org.apache.drill.exec.vector.UntypedNullHolder; import org.apache.drill.exec.vector.complex.reader.FieldReader; import org.apache.drill.exec.vector.complex.writer.BaseWriter.ListWriter; import org.apache.drill.exec.vector.complex.writer.FieldWriter; @@ -81,6 +82,16 @@ abstract class AbstractBaseReader implements FieldReader{ } @Override + public void read(UntypedNullHolder holder) { + throw new IllegalStateException("The current reader doesn't support reading untyped null"); + } + + @Override + public void read(int index, UntypedNullHolder holder) { + throw new IllegalStateException("The current reader doesn't support reading untyped null"); + } + + @Override public void copyAsValue(UnionWriter writer) { throw new IllegalStateException("The current reader doesn't support reading union type"); } diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedHolderReaderImpl.java similarity index 56% copy from exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java copy to exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedHolderReaderImpl.java index dac0f0d..3e10d78 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedHolderReaderImpl.java @@ -15,15 +15,37 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.exec.vector.complex.reader; +package org.apache.drill.exec.vector.complex.impl; -import org.apache.drill.exec.vector.complex.reader.BaseReader.ListReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.MapReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedListReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedMapReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.ScalarReader; +import org.apache.drill.common.types.TypeProtos; +import org.apache.drill.exec.vector.UntypedNullHolder; +public class UntypedHolderReaderImpl extends AbstractFieldReader { + private final UntypedNullHolder holder; -public interface FieldReader extends MapReader, ListReader, ScalarReader, RepeatedMapReader, RepeatedListReader { -} \ No newline at end of file + public UntypedHolderReaderImpl(UntypedNullHolder holder) { + this.holder = holder; + } + + @Override + public int size() { + return 0; + } + + @Override + public boolean next() { + return false; + } + + @Override + public TypeProtos.MajorType getType() { + return holder.getType(); + } + + @Override + public boolean isSet() { + return false; + } + +} diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReader.java similarity index 59% copy from exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java copy to exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReader.java index dac0f0d..f15b852 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReader.java @@ -15,15 +15,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.exec.vector.complex.reader; +package org.apache.drill.exec.vector.complex.impl; -import org.apache.drill.exec.vector.complex.reader.BaseReader.ListReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.MapReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedListReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedMapReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.ScalarReader; +import org.apache.drill.exec.vector.UntypedNullHolder; +import org.apache.drill.exec.vector.complex.reader.BaseReader; +public interface UntypedReader extends BaseReader { + boolean isSet(); + int size(); + void read(UntypedNullHolder holder); + void read(int arrayIndex, UntypedNullHolder holder); -public interface FieldReader extends MapReader, ListReader, ScalarReader, RepeatedMapReader, RepeatedListReader { -} \ No newline at end of file +} diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReaderImpl.java similarity index 56% copy from exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java copy to exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReaderImpl.java index dac0f0d..da6f63e 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReaderImpl.java @@ -15,15 +15,36 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.exec.vector.complex.reader; +package org.apache.drill.exec.vector.complex.impl; -import org.apache.drill.exec.vector.complex.reader.BaseReader.ListReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.MapReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedListReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedMapReader; -import org.apache.drill.exec.vector.complex.reader.BaseReader.ScalarReader; +import org.apache.drill.common.types.TypeProtos; +import org.apache.drill.exec.vector.UntypedNullHolder; +public class UntypedReaderImpl extends AbstractFieldReader { + @Override + public TypeProtos.MajorType getType() { + return UntypedNullHolder.TYPE; + } -public interface FieldReader extends MapReader, ListReader, ScalarReader, RepeatedMapReader, RepeatedListReader { -} \ No newline at end of file + @Override + public boolean isSet() { + return false; + } + + @Override + public int size() { + return 0; + } + + @Override + public void read(UntypedNullHolder holder) { + holder.isSet = 0; + } + + @Override + public void read(int arrayIndex, UntypedNullHolder holder) { + holder.isSet = 0; + } + +} diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java index dac0f0d..488306e 100644 --- a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java +++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/reader/FieldReader.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.vector.complex.reader; +import org.apache.drill.exec.vector.complex.impl.UntypedReader; import org.apache.drill.exec.vector.complex.reader.BaseReader.ListReader; import org.apache.drill.exec.vector.complex.reader.BaseReader.MapReader; import org.apache.drill.exec.vector.complex.reader.BaseReader.RepeatedListReader; @@ -25,5 +26,5 @@ import org.apache.drill.exec.vector.complex.reader.BaseReader.ScalarReader; -public interface FieldReader extends MapReader, ListReader, ScalarReader, RepeatedMapReader, RepeatedListReader { +public interface FieldReader extends MapReader, ListReader, ScalarReader, RepeatedMapReader, RepeatedListReader, UntypedReader { } \ No newline at end of file