[Impala-ASF-CR] IMPALA-5994: Failure in star expansion on struct fields

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8169 )

Change subject: IMPALA-5994: Failure in star expansion on struct fields
..


Patch Set 2: Code-Review+2

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/8169/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-5994: Failure in star expansion on struct fields
Commit msg should describe the change/fix, not the symptom of the bug. For 
example this would be clearer:

IMPALA-5994: Lower case struct-field names


http://gerrit.cloudera.org:8080/#/c/8169/2/fe/src/main/java/org/apache/impala/catalog/StructField.java
File fe/src/main/java/org/apache/impala/catalog/StructField.java:

http://gerrit.cloudera.org:8080/#/c/8169/2/fe/src/main/java/org/apache/impala/catalog/StructField.java@37
PS2, Line 37: // Impala expects column names to be lower cased, but this is 
not always true for
// Impala expects field names to be in lower case, but type strings stored in 
the HMS are not guaranteed to be lower case.

(no need to mention JIRA imo)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
Gerrit-Change-Number: 8169
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 28 Sep 2017 23:14:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5994: Failure in star expansion on struct fields

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8169 )

Change subject: IMPALA-5994: Failure in star expansion on struct fields
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
Gerrit-Change-Number: 8169
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 28 Sep 2017 22:36:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5994: Failure in star expansion on struct fields

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8169 )

Change subject: IMPALA-5994: Failure in star expansion on struct fields
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8169/1/fe/src/main/java/org/apache/impala/catalog/StructField.java
File fe/src/main/java/org/apache/impala/catalog/StructField.java:

http://gerrit.cloudera.org:8080/#/c/8169/1/fe/src/main/java/org/apache/impala/catalog/StructField.java@37
PS1, Line 37: // Impala expects column names to be lower cased, but this is 
not always true for
> Can you add a brief comment why we need to do this?
Done


http://gerrit.cloudera.org:8080/#/c/8169/1/tests/query_test/test_nested_types.py
File tests/query_test/test_nested_types.py:

http://gerrit.cloudera.org:8080/#/c/8169/1/tests/query_test/test_nested_types.py@96
PS1, Line 96: rCase
> nit: This seems easy to miss. Can you pick something more obvious, like cam
Done


http://gerrit.cloudera.org:8080/#/c/8169/1/tests/query_test/test_nested_types.py@100
PS1, Line 100: U
> Does the uppercase F matter here?
Not really. This query passed even before the fix, just adding a little extra 
coverage while I'm here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
Gerrit-Change-Number: 8169
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 28 Sep 2017 22:27:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5994: Failure in star expansion on struct fields

[Thomas Tauber-Marshall (Code Review)]

Hello Lars Volker, Alex Behm,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8169

to look at the new patch set (#2).

Change subject: IMPALA-5994: Failure in star expansion on struct fields
..

IMPALA-5994: Failure in star expansion on struct fields

Impala tries to always store column names in lower case. As part of a
cleanup of issues related to upper case Kudu column names, a check was
added in Analyzer to enforce this.

The check fails when doing star expansion on a struct to select all
fields in the case where a table was created in Hive with upper case
letters in a struct field name. This happens because Hive does not
covert struct field names to all lower case in HMS.

The solution is to force StructField names to lower case.

Testing:
- Added a test in test_nested_types.py

Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
---
M fe/src/main/java/org/apache/impala/catalog/StructField.java
M tests/query_test/test_nested_types.py
2 files changed, 18 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/8169/2
--
To view, visit http://gerrit.cloudera.org:8080/8169
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
Gerrit-Change-Number: 8169
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-5994: Failure in star expansion on struct fields

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8169 )

Change subject: IMPALA-5994: Failure in star expansion on struct fields
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8169/1/fe/src/main/java/org/apache/impala/catalog/StructField.java
File fe/src/main/java/org/apache/impala/catalog/StructField.java:

http://gerrit.cloudera.org:8080/#/c/8169/1/fe/src/main/java/org/apache/impala/catalog/StructField.java@37
PS1, Line 37: name_ = name.toLowerCase();
Can you add a brief comment why we need to do this?


http://gerrit.cloudera.org:8080/#/c/8169/1/tests/query_test/test_nested_types.py
File tests/query_test/test_nested_types.py:

http://gerrit.cloudera.org:8080/#/c/8169/1/tests/query_test/test_nested_types.py@96
PS1, Line 96: fielD
nit: This seems easy to miss. Can you pick something more obvious, like 
camelField?


http://gerrit.cloudera.org:8080/#/c/8169/1/tests/query_test/test_nested_types.py@100
PS1, Line 100: F
Does the uppercase F matter here?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
Gerrit-Change-Number: 8169
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 28 Sep 2017 21:58:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5994: Failure in star expansion on struct fields

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8169


Change subject: IMPALA-5994: Failure in star expansion on struct fields
..

IMPALA-5994: Failure in star expansion on struct fields

Impala tries to always store column names in lower case. As part of a
cleanup of issues related to mixed case Kudu column names, a check was
added in Analyzer to enforce this.

The check fails when doing star expansion on a struct to select all
fields in the case where a table was created in Hive with upper case
letters in a struct field name. This happens because Hive does not
covert struct field names to all lower case in HMS.

The solution is to force StructField names to lower case.

Testing:
- Added a test in test_nested_types.py

Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
---
M fe/src/main/java/org/apache/impala/catalog/StructField.java
M tests/query_test/test_nested_types.py
2 files changed, 16 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/8169/1
--
To view, visit http://gerrit.cloudera.org:8080/8169
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iacd9714ac2301a55ee8b64f0102f6f156fb0370e
Gerrit-Change-Number: 8169
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall