Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17847 )
Change subject: IMPALA-10838: Error when struct returned from WITH() ...................................................................... Patch Set 15: (6 comments) Looks good to me! http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java File fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java: http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java@239 PS14, Line 239: // Struct children are allowed to be non-materialised because the query may only : // concern a subset of the fields of the struct. > It is not the child but the struct that we keep. I see. Thanks for the explanation. So the purpose of the IF test below at line 242 is to retain any structs which may have a reference in the query. But would it be possible that some of the structs do not have any references in the query? If so, we still can remove them. http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/Path.java File fe/src/main/java/org/apache/impala/analysis/Path.java: http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/Path.java@480 PS14, Line 480: prefixPath.size() > List.subList would throw an exception but we check it on L479 so it is guar Done http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java File fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java: http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@186 PS14, Line 186: if (tupleDescs != null) > In the WHILE loop we check 'tupleDesc' of type TupleDescriptor, here it is Aha. my bad. Maybe rename tupleDesc as parentTupleDesc to avoid confusion? http://gerrit.cloudera.org:8080/#/c/17847/14/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@188 PS14, Line 188: parentStructSl > Renamed it to parentStructSlotDesc as I think it is important that it shoul Done http://gerrit.cloudera.org:8080/#/c/17847/15/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test File testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test: http://gerrit.cloudera.org:8080/#/c/17847/15/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test@187 PS15, Line 187: , sub.id I wonder if sub.id can be removed from the order clause to fit the test description better. http://gerrit.cloudera.org:8080/#/c/17847/15/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test@198 PS15, Line 198: # WITH clause creates an inline view containing a nested struct; filter by a struct field : # from the inline view. : with sub as ( : select id, outer_struct from functional_orc_def.complextypes_nested_structs) : select sub.id, sub.outer_struct.str : from sub : where length(sub.outer_struct.str) < 4; May also add tests as follows. 1. The main query does not select from the inline view at all; 2. The inline return two structs and the main query refers to only one struct. -- To view, visit http://gerrit.cloudera.org:8080/17847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iadb9233677355b85d424cc3f22b00b5a3bf61c57 Gerrit-Change-Number: 17847 Gerrit-PatchSet: 15 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Comment-Date: Mon, 14 Feb 2022 15:23:53 +0000 Gerrit-HasComments: Yes
