[Impala-ASF-CR] WiP: IMPALA-9495: Support struct in select list for ORC tables

["Qifan Chen (Code Review)" <ge...@cloudera.org>]

Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17638 )

Change subject: WiP: IMPALA-9495: Support struct in select list for ORC tables
......................................................................


Patch Set 4:

(10 comments)

Looks good!

http://gerrit.cloudera.org:8080/#/c/17638/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17638/4//COMMIT_MSG@44
PS4, Line 44: the 
nit. "a row in a row batch"


http://gerrit.cloudera.org:8080/#/c/17638/4//COMMIT_MSG@58
PS4, Line 58: Restrictions:
Since this patch lays the foundation work for STRUCT support (even though the title of the commit is about STRUCT in select list), I wonder if we can formally describe where  STRUCTs can appear in DDL and DML. For example

1. In CREATE TABLE DDL;
2. In LOAD statement;
3. IN SELECT statement:
     a), in SELECT list;
     b), as a virtual table in JOIN;
     c), ... ...


http://gerrit.cloudera.org:8080/#/c/17638/4//COMMIT_MSG@64
PS4, Line 64: 
nit. May add items into the test section.


http://gerrit.cloudera.org:8080/#/c/17638/4/be/src/service/query-result-set.cc
File be/src/service/query-result-set.cc:

http://gerrit.cloudera.org:8080/#/c/17638/4/be/src/service/query-result-set.cc@406
PS4, Line 406: DCHECK(col_input.columnType.types.size() > 0);
This line probably should be moved to the block for STRUCT. In the ELSE block, we have the assertion for scalar type.


http://gerrit.cloudera.org:8080/#/c/17638/4/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/17638/4/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test@56
PS4, Line 56: 
would be nice to add more cases.

1. select with where clause on the id column;
2. select with where cause on the outer_struct with some SQL string function to qualify a particular field in the struct;
3. select with outer_struct in the select list in a subquery;

I also wonder if it is possible to select a particular field from a struct (as the whole thing).


http://gerrit.cloudera.org:8080/#/c/17638/4/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test
File testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test:

http://gerrit.cloudera.org:8080/#/c/17638/4/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test@4
PS4, Line 4: select id, tiny_struct from functional_orc_def.complextypes_structs;
nit. Is it possible to say this? If so, I'd like to suggest that we add a couple test queries referencing struct members.

Depending on the type of the struct member, we could also apply SQL functions. 

select id, tiny_struct from functional_orc_def.complextypes_structs my_struct
where my_struct.b = false


http://gerrit.cloudera.org:8080/#/c/17638/4/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test@17
PS4, Line 17:  order by id;
nit. Can we add one with group by on tiny_struct? Is it supported?


http://gerrit.cloudera.org:8080/#/c/17638/4/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test@153
PS4, Line 153: and order by a member of the struct
nit. Should remove this? I did not see order by clause in the query.


http://gerrit.cloudera.org:8080/#/c/17638/4/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test@263
PS4, Line 263: joins
nit. Can we add other join flavors such as equi join or left join?


http://gerrit.cloudera.org:8080/#/c/17638/4/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test@266
PS4, Line 266: tbl,
             :     tbl.nested_struct.c.d as outer_arr, outer_arr.ITEM as inner_arr;
nit. Maybe add a comment on the join columns selected from the three tables.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fbe56bdcd372b72e99c0195d87a818e7fa4bc3a
Gerrit-Change-Number: 17638
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Aug 2021 18:31:32 +0000
Gerrit-HasComments: Yes