Christopher Channing has posted comments on this change. Change subject: IMPALA-3938: Prevent illegal implicit collection references being used. ......................................................................
Patch Set 1: (3 comments) I made the changes that you requested. 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() Sorry, not sure how I missed that. I tried out your suggestion, but it did not catch the following broken query: "select key from test_table.locations" (same behaviour that the bug exhibits). For this broken scenario, the flag @ L219 could be set if the type is a collection and we look at the optional type, but it would mean duplicating a little code to recreate the structure (in order to get the optional field). Instead of that approach, I've reworked this a little to populate expectExplicitMatch inside the loop when we're dealing with a hierarchy of nested collections. 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 Done Line 467: "Could not resolve column/field reference: 'm.value'"); > whitespace Done -- 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-Reviewer: Christopher Channing <[email protected]> Gerrit-HasComments: Yes
