Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17038 )

Change subject: IMPALA-10482: Select-star query on unrelative collection column 
of transactional table hits IllegalStateException
......................................................................


Patch Set 2:

(7 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/17038/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17038/1//COMMIT_MSG@7
PS1, Line 7: IMPAL
> nit: IMPALA
Oops, thanks. Done.


http://gerrit.cloudera.org:8080/#/c/17038/1//COMMIT_MSG@25
PS1, Line 25: when doing star expansion
> I think we also need to ignore them when resolving slot refs. These cases a
Good catch. Added fix and tests in PS2.


http://gerrit.cloudera.org:8080/#/c/17038/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/17038/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@130
PS1, Line 130:   SelectStmt(SelectList selectList,
             :              FromClause fromClause,
             :              Expr wherePred
> Maybe the current name is ok. We may refactor the AcidRewriter and reuse it
Made this a property of TableRef, the name is 'isHidden()'.


http://gerrit.cloudera.org:8080/#/c/17038/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@130
PS1, Line 130:   SelectStmt(SelectList selectList,
             :              FromClause fromClause,
             :              Expr wherePred
> I am not too familiar with the full ACID rewrites, but my guess is that the
You are right, but since then Quanlong mentioned that we will get hidden table 
refs for other reasons as well.


http://gerrit.cloudera.org:8080/#/c/17038/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@133
PS1, Line 133: ngPredicate, Lis
> optional: I think that it would be slightly better to store this informatio
Done


http://gerrit.cloudera.org:8080/#/c/17038/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-scanner-basic.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-scanner-basic.test:

http://gerrit.cloudera.org:8080/#/c/17038/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-scanner-basic.test@317
PS1, Line 317: ====
> +1
Sure.


http://gerrit.cloudera.org:8080/#/c/17038/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-scanner-basic.test@317
PS1, Line 317: ====
> Can you add a bit more complex query? The idea is to include the same hidde
This query actually revealed another bug, opened IMPALA-10493 for it.

But if we put put the JOIN condition to the WHERE clause instead of the ON 
clause the query works fine.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fc758d3c1e75c7066936d590aec8bff8d2b00b0
Gerrit-Change-Number: 17038
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 09 Feb 2021 12:51:12 +0000
Gerrit-HasComments: Yes

Reply via email to