You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by db...@apache.org on 2022/11/04 12:09:08 UTC
[impala] branch master updated: IMPALA-11687: Select * with EXPAND_COMPLEX_TYPES=1 and explicit complex types fails
This is an automated email from the ASF dual-hosted git repository.
dbecker pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 77dc20264 IMPALA-11687: Select * with EXPAND_COMPLEX_TYPES=1 and explicit complex types fails
77dc20264 is described below
commit 77dc20264cd4d0063f249224d80a3dfb2721fb36
Author: Daniel Becker <da...@cloudera.com>
AuthorDate: Wed Oct 26 18:17:19 2022 +0200
IMPALA-11687: Select * with EXPAND_COMPLEX_TYPES=1 and explicit complex types fails
If EXPAND_COMPLEX_TYPES is set to true, some queries that combine star
expressions and explicitly given complex columns fail:
select outer_struct, * from
functional_orc_def.complextypes_nested_structs;
ERROR: IllegalStateException: Illegal reference to non-materialized
slot: tid=1 sid=1
select *, outer_struct.str from
functional_orc_def.complextypes_nested_structs;
ERROR: IllegalStateException: null
Having two stars in a table with complex columns also fails.
select *, * from functional_orc_def.complextypes_nested_structs;
ERROR: IllegalStateException: Illegal reference to non-materialized
slot: tid=6 sid=13
The error is because of this line in 'SelectStmt.addStarResultExpr()':
https://github.com/apache/impala/blob/8e350d0a8a7c95810c7704c8a5264a60daed783d/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java#L811
What we want to do is create 'SlotRef's for the struct children
(recursively) but 'reExpandStruct()' also creates new 'SlotDescriptor's
for the children. The new 'SlotDescriptor's are redundant and are not
inserted into the tree which can leave them unmaterialised or without a
correct memory layout.
The solution is to only create the 'SlotRef's for the struct children
without creating new 'SlotDescriptor's. This leads us to another
problem:
- for structs, it is 'SlotRef.analyzeImpl()' that creates the child
'SlotRef's
- the constructor 'SlotRef(SlotDescriptor desc)' sets 'isAnalyzed_' to
true.
Before structs were allowed, this was correct but now struct-typed
'SlotRef's created with the above constructor are counted as analysed
but lack child expressions, which would have been added if 'analyze()'
had been called on them. This essentially violates the contract of this
constructor.
This commit modifies 'SlotRef(SlotDescriptor desc)' so that child
expressions are generated for structs, restoring the correct semantics
of this constructor. After this, it is no longer necessary to call
'reExpandStruct()' in 'SelectStmt.addStarResultExpr()'.
Testing:
- Added the failing test cases and a few variations of them to
nested-types-star-expansion.test
Change-Id: Ia8cf53b0a7409faca668713228bfef275f3833f9
Reviewed-on: http://gerrit.cloudera.org:8080/19171
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../org/apache/impala/analysis/SelectStmt.java | 14 +++---
.../java/org/apache/impala/analysis/SlotRef.java | 33 ++++++++-----
.../QueryTest/nested-types-star-expansion.test | 57 ++++++++++++++++++++++
3 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index b94ff2494..b75b86da9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -713,9 +713,10 @@ public class SelectStmt extends QueryStmt {
}
/**
- * Expand "*" select list item, ignoring semi-joined tables as well as
- * complex-typed fields because those are currently illegal in any select
- * list (even for inline views, etc.)
+ * Expand "*" select list item, ignoring semi-joined tables because those are
+ * currently illegal in any select list (even for inline views, etc.). Also ignores
+ * complex-typed fields for backwards compatibility unless EXPAND_COMPLEX_TYPES is set
+ * to true.
*/
private void expandStar() throws AnalysisException {
if (fromClause_.isEmpty()) {
@@ -734,8 +735,8 @@ public class SelectStmt extends QueryStmt {
}
/**
- * Expand "path.*" from a resolved path, ignoring complex-typed fields
- * for backwards compatibility.
+ * Expand "path.*" from a resolved path, ignoring complex-typed fields for backwards
+ * compatibility unless EXPAND_COMPLEX_TYPES is set to true.
*/
private void expandStar(Path resolvedPath)
throws AnalysisException {
@@ -806,9 +807,6 @@ public class SelectStmt extends QueryStmt {
Preconditions.checkState(slotRef.isAnalyzed(),
"Analysis should be done in constructor");
- if(slotRef.getType().isStructType()){
- slotRef.reExpandStruct(analyzer_);
- }
// Empty matched types means this is expanded from star of a catalog table.
// For star of complex types, e.g. my_struct.*, my_array.*, my_map.*, the matched
// types will have the complex type so it's not empty.
diff --git a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
index 3cdace050..7297fa8d9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
@@ -88,6 +88,9 @@ public class SlotRef extends Expr {
String alias = desc.getParent().getAlias();
label_ = (alias != null ? alias + "." : "") + desc.getLabel();
numDistinctValues_ = adjustNumDistinctValues();
+
+ if (type_.isStructType()) addStructChildrenAsSlotRefs();
+
analysisDone();
}
@@ -191,7 +194,10 @@ public class SlotRef extends Expr {
}
}
}
- if (type_.isStructType()) addStructChildrenAsSlotRefs(analyzer);
+ if (type_.isStructType()) {
+ addStructChildrenAsSlotRefs();
+ checkForUnsupportedStructFields();
+ }
}
/**
@@ -205,15 +211,16 @@ public class SlotRef extends Expr {
children_.clear();
analyzer.createStructTuplesAndSlotDescs(desc_);
- addStructChildrenAsSlotRefs(analyzer);
+ addStructChildrenAsSlotRefs();
+ checkForUnsupportedStructFields();
}
- // Expects the type of this SlotRef as a StructType. Throws an AnalysisException if any
- // of the struct fields of this Slot ref is a collection or unsupported type.
- private void checkForUnsupportedFieldsForStruct() throws AnalysisException {
+ // Throws an AnalysisException if any of the struct fields, recursively, of this SlotRef
+ // is a collection or unsupported type. Should only be used if this is a struct.
+ private void checkForUnsupportedStructFields() throws AnalysisException {
Preconditions.checkState(type_ instanceof StructType);
- for (StructField structField : ((StructType)type_).getFields()) {
- final Type fieldType = structField.getType();
+ for (Expr child : getChildren()) {
+ final Type fieldType = child.getType();
if (!fieldType.isSupported()) {
throw new AnalysisException("Unsupported type '"
+ fieldType.toSql() + "' in '" + toSql() + "'.");
@@ -226,22 +233,24 @@ public class SlotRef extends Expr {
throw new AnalysisException("Struct containing a BINARY type is not " +
"allowed in the select list (IMPALA-11491).");
}
+
+ if (fieldType.isStructType()) {
+ Preconditions.checkState(child instanceof SlotRef);
+ ((SlotRef) child).checkForUnsupportedStructFields();
+ }
}
}
// Assumes this 'SlotRef' is a struct and that desc_.itemTupleDesc_ has already been
// filled. Creates the children 'SlotRef's for the struct recursively.
- private void addStructChildrenAsSlotRefs(Analyzer analyzer) throws AnalysisException {
+ private void addStructChildrenAsSlotRefs() {
Preconditions.checkState(desc_.getType().isStructType());
- checkForUnsupportedFieldsForStruct();
TupleDescriptor structTuple = desc_.getItemTupleDesc();
Preconditions.checkState(structTuple != null);
for (SlotDescriptor childSlot : structTuple.getSlots()) {
+ // If 'childSlot' is also a struct, the constructor will call this method on it.
SlotRef childSlotRef = new SlotRef(childSlot);
children_.add(childSlotRef);
- if (childSlot.getType().isStructType()) {
- childSlotRef.addStructChildrenAsSlotRefs(analyzer);
- }
}
}
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-types-star-expansion.test b/testdata/workloads/functional-query/queries/QueryTest/nested-types-star-expansion.test
index 8434252ea..1bb8985fb 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-types-star-expansion.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-types-star-expansion.test
@@ -162,3 +162,60 @@ where s.id = a.id and s.id = m.id and s.id = 3
---- TYPES
INT, STRING, INT, BIGINT
====
+---- QUERY
+# Combining star expansion with an explicitly given struct.
+set EXPAND_COMPLEX_TYPES=true;
+select outer_struct, * from functional_orc_def.complextypes_nested_structs where id = 1;
+---- RESULTS
+'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}',1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}'
+---- TYPES
+STRING, INT, STRING
+====
+---- QUERY
+# Combining star expansion with an explicitly given struct.
+set EXPAND_COMPLEX_TYPES=true;
+select *, outer_struct from functional_orc_def.complextypes_nested_structs where id = 1;
+---- RESULTS
+1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}','{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}'
+---- TYPES
+INT, STRING, STRING
+====
+---- QUERY
+# Combining star expansion with an explicitly given struct field.
+set EXPAND_COMPLEX_TYPES=true;
+select *, outer_struct.str
+from functional_orc_def.complextypes_nested_structs where id = 1;
+---- RESULTS
+1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}','somestr1'
+---- TYPES
+INT, STRING, STRING
+====
+---- QUERY
+# Combining star expansion with an explicitly given struct field.
+set EXPAND_COMPLEX_TYPES=true;
+select outer_struct.str, *
+from functional_orc_def.complextypes_nested_structs where id = 1;
+---- RESULTS
+'somestr1',1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}'
+---- TYPES
+STRING, INT, STRING
+====
+---- QUERY
+# Combining star expansion with explicitly given structs and struct fields.
+set EXPAND_COMPLEX_TYPES=true;
+select outer_struct.inner_struct1, *, outer_struct, outer_struct.str
+from functional_orc_def.complextypes_nested_structs where id = 1;
+---- RESULTS
+'{"str":"somestr2","de":12345.12}',1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}','{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}','somestr1'
+---- TYPES
+STRING, INT, STRING, STRING, STRING
+====
+---- QUERY
+# Two stars including complex types.
+set EXPAND_COMPLEX_TYPES=true;
+select *, * from functional_orc_def.complextypes_nested_structs where id = 1;
+---- RESULTS
+1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}',1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}'
+---- TYPES
+INT, STRING, INT, STRING
+====