Alex Behm has posted comments on this change.

Change subject: IMPALA-3938: Prevent illegal implicit collection references 
being used.
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4079/1/fe/src/main/java/com/cloudera/impala/analysis/Path.java
File fe/src/main/java/com/cloudera/impala/analysis/Path.java:

Line 232:           if (!structType.isMapType() && field.getType() instanceof 
MapType) {
how could structType every be a map type? do you mean to call isMapStruct() 
declared in CollectionStructType?

it would be great if we could phrase this new constraint with the existing 
expectExplicitMatch mechanism, e.g., maybe we can set expectExplicitMatch=true 
in the if-block at L255?


http://gerrit.cloudera.org:8080/#/c/4079/1/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java:

Line 438:     // Array of structs and maps with name conflicts. Both implicit 
and explicit
your new tests don't really belong in this test 'cluster' because there are no 
name conflicts

I think the new tests belong at the very end, they are similar in spirit to the 
very last test (test constraints when implicit matches are ok). A similar 
explanation would be helpful as well.

please add a new test table t9


Line 467:         "Could not resolve column/field reference: 'm.value'");    
whitespace


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I703c7e9c8c9c95c9fee714767367373eec10cd0c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Christopher Channing <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-HasComments: Yes

Reply via email to