You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Daniel Becker (Code Review)" <ge...@cloudera.org> on 2022/10/27 12:37:07 UTC

[Impala-ASF-CR] IMPALA-11687: Select * with EXPAND COMPLEX TYPES=1 and explicit complex types fails

Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19171


Change subject: IMPALA-11687: Select * with EXPAND_COMPLEX_TYPES=1 and explicit complex types fails
......................................................................

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.
Befor 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
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M testdata/workloads/functional-query/queries/QueryTest/nested-types-star-expansion.test
3 files changed, 84 insertions(+), 20 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/19171/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19171
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8cf53b0a7409faca668713228bfef275f3833f9
Gerrit-Change-Number: 19171
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>

[Impala-ASF-CR] IMPALA-11687: Select * with EXPAND COMPLEX TYPES=1 and explicit complex types fails

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.

Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19171 )

Change subject: IMPALA-11687: Select * with EXPAND_COMPLEX_TYPES=1 and explicit complex types fails
......................................................................

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
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M testdata/workloads/functional-query/queries/QueryTest/nested-types-star-expansion.test
3 files changed, 84 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/19171/3
-- 
To view, visit http://gerrit.cloudera.org:8080/19171
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia8cf53b0a7409faca668713228bfef275f3833f9
Gerrit-Change-Number: 19171
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-11687: Select * with EXPAND COMPLEX TYPES=1 and explicit complex types fails

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.

Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/19171 )

Change subject: IMPALA-11687: Select * with EXPAND_COMPLEX_TYPES=1 and explicit complex types fails
......................................................................


Patch Set 2: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19171
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8cf53b0a7409faca668713228bfef275f3833f9
Gerrit-Change-Number: 19171
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Nov 2022 16:17:23 +0000
Gerrit-HasComments: No