arina-ielchiieva closed pull request #1505: DRILL-6797: Fix UntypedNull
handling for complex types
URL: https://github.com/apache/drill/pull/1505
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java
b/exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java
index 8c8097d3e0e..7357243a890 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 static FieldConverter getConverter(RecordWriter
recordWriter, int fieldId
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 e0f34e8b9c3..8685130e23e 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 HoldingContainer visitUnknown(LogicalExpression e,
ClassGenerator<?> gene
} 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 00000000000..fa2765c1992
--- /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 6affae1c628..45233c4da4e 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 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 @@ protected PrimitiveType getPrimitiveType(MaterializedField
field) {
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 @@ private Type getType(MaterializedField field) {
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 00000000000..4976947eaa2
--- /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 4e5856b2279..4a763c211eb 100644
--- a/exec/vector/src/main/codegen/templates/AbstractFieldReader.java
+++ b/exec/vector/src/main/codegen/templates/AbstractFieldReader.java
@@ -103,6 +103,10 @@ public void copyAsField(String name, ${name}Writer writer)
{
}
</#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 02e1c279071..430a41b431b 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 static int getSize(MajorType major) {
}
</#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 90356c11f65..b65f866edc8 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 @@
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 f7536d89690..9dd8480cb9a 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 UntypedNullVector(MaterializedField field,
BufferAllocator allocator) {
}
@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 7c46f0b5f20..668a332664f 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.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;
@@ -80,6 +81,16 @@ public void read(int index, UnionHolder holder) {
throw new IllegalStateException("The current reader doesn't support
reading union type");
}
+ @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/impl/UntypedHolderReaderImpl.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedHolderReaderImpl.java
new file mode 100644
index 00000000000..3e10d7848c3
--- /dev/null
+++
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedHolderReaderImpl.java
@@ -0,0 +1,51 @@
+/*
+ * 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.vector.complex.impl;
+
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.exec.vector.UntypedNullHolder;
+
+public class UntypedHolderReaderImpl extends AbstractFieldReader {
+
+ private final UntypedNullHolder holder;
+
+ 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/impl/UntypedReader.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReader.java
new file mode 100644
index 00000000000..f15b8520e91
--- /dev/null
+++
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReader.java
@@ -0,0 +1,30 @@
+/*
+ * 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.vector.complex.impl;
+
+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);
+
+}
diff --git
a/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReaderImpl.java
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReaderImpl.java
new file mode 100644
index 00000000000..da6f63ed212
--- /dev/null
+++
b/exec/vector/src/main/java/org/apache/drill/exec/vector/complex/impl/UntypedReaderImpl.java
@@ -0,0 +1,50 @@
+/*
+ * 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.vector.complex.impl;
+
+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;
+ }
+
+ @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 dac0f0d5e6c..488306ec021 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 @@
-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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
