You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2016/10/27 02:43:35 UTC
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Thomas Tauber-Marshall has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4861
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
IMPALA-4372: 'Describe formatted' returns types in upper case
A recent change caused 'describe formatted' to display the types
in all upper case, but we want 'describe formatted' to match Hive's
'describe' output, which displays the types in lower case.
Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
---
M fe/src/main/java/org/apache/impala/catalog/Column.java
M testdata/workloads/functional-query/queries/QueryTest/avro-schema-changes.test
2 files changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/4861/1
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4861/2/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:
PS2, Line 84: results,
> This only impacts the comparison of NULL vs. empty string. I expanded the a
Sorry maybe I am missing something here, but isn't lower() applied on the entire impala and hive output. So my understanding is that if Impala returns let's say 'STRING' and and Hive returns 'string', then by calling lower() the output will be the same and we will not catch inconsistencies in the generated output.
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has uploaded a new patch set (#2).
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
IMPALA-4372: 'Describe formatted' returns types in upper case
A recent change caused 'describe formatted' to display the types
in all upper case, but we want 'describe formatted' to match Hive's
'describe' output, which displays the types in lower case.
This patch also fixes several problems with test_describe_formatted,
which was encountering an error but reporting success.
Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
---
M fe/src/main/java/org/apache/impala/catalog/Column.java
M testdata/workloads/functional-query/queries/QueryTest/avro-schema-changes.test
M tests/common/impala_test_suite.py
M tests/metadata/test_metadata_query_statements.py
M tests/performance/query_exec_functions.py
M tests/performance/query_executor.py
6 files changed, 43 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/4861/2
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
IMPALA-4372: 'Describe formatted' returns types in upper case
A recent change caused 'describe formatted' to display the types
in all upper case, but we want 'describe formatted' to match Hive's
'describe' output, which displays the types in lower case.
This patch also fixes several problems with test_describe_formatted,
which was encountering an error but reporting success.
Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Reviewed-on: http://gerrit.cloudera.org:8080/4861
Reviewed-by: Dimitris Tsirogiannis <dt...@cloudera.com>
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/catalog/Column.java
M testdata/workloads/functional-query/queries/QueryTest/avro-schema-changes.test
M tests/common/impala_test_suite.py
M tests/metadata/test_metadata_query_statements.py
M tests/performance/query_exec_functions.py
M tests/performance/query_executor.py
6 files changed, 49 insertions(+), 26 deletions(-)
Approvals:
Internal Jenkins: Verified
Dimitris Tsirogiannis: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has uploaded a new patch set (#3).
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
IMPALA-4372: 'Describe formatted' returns types in upper case
A recent change caused 'describe formatted' to display the types
in all upper case, but we want 'describe formatted' to match Hive's
'describe' output, which displays the types in lower case.
This patch also fixes several problems with test_describe_formatted,
which was encountering an error but reporting success.
Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
---
M fe/src/main/java/org/apache/impala/catalog/Column.java
M testdata/workloads/functional-query/queries/QueryTest/avro-schema-changes.test
M tests/common/impala_test_suite.py
M tests/metadata/test_metadata_query_statements.py
M tests/performance/query_exec_functions.py
M tests/performance/query_executor.py
6 files changed, 49 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/4861/3
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4861/2/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:
PS2, Line 84: results,
> Sorry maybe I am missing something here, but isn't lower() applied on the e
RIght, except that after calling lower() it never directly compares the two strings to each other again, it just compares them to 'null' and ''.
So if Impala return 'STRING' and Hive return 'string', they won't match, then we'lll call replace() and lower(), and convert the impala output to 'string' and then 'not ((impala == "'null'" and hive == "''") or (impala == "''" and hive == "'null'"))' will be false and we'll return false.
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 1:
How come this discrepancy between Impala's and Hive's output wasn't caught by test_describe_formatted in test_metadata_query_statements.py?
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 2:
test_describe_formatted did not catch this bug because it was silently failing.
The primary issue there was that it wasn't passing a 'transport' param to the jdbc client, and it uses the workload runner infrastructure, which logs errors it encounters but doesn't fail tests (in query_exec_functions.py:run_query_capture_results). To prevent other tests from making the same mistake, I added an assert in JdbcQueryExecConfig that ensures that the transport is set.
There was also an incorrect number of parameters being passed to the HiveQueryResult constructor in create_exec_result that I fixed.
Finally, it turns out that we handle NULLs differently from Hive. For now, I've left our NULL behavior as is, and just modified the test to allow NULLs to equal empty strings, which also allowed me to remove an xfail from the test.
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4861/2/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:
PS2, Line 84: .lower()
Is this really needed? I believe by doing this, the output of Impala will always match that from Hive irrespective of how each system treats case, which kind of defies the purpose of this test, no?
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4861/2/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:
PS2, Line 84: results,
> RIght, except that after calling lower() it never directly compares the two
Right, thanks for the explanation.
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4372: 'Describe formatted' returns types in upper case
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4372: 'Describe formatted' returns types in upper case
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4861/2/tests/metadata/test_metadata_query_statements.py
File tests/metadata/test_metadata_query_statements.py:
PS2, Line 84: results,
> Is this really needed? I believe by doing this, the output of Impala will a
This only impacts the comparison of NULL vs. empty string. I expanded the above comment to explain this.
--
To view, visit http://gerrit.cloudera.org:8080/4861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274b97d4d1247244247fb38a5ca7f4c10bba8d22
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes