impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2636: HS2 GetTables() returns TABLE TYPE as TABLE for VIEW
Date Sat, 30 Sep 2017 03:13:31 GMT
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/7353 )

Change subject: IMPALA-2636: HS2 GetTables() returns TABLE_TYPE as TABLE for VIEW
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:

http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/main/java/org/apache/impala/service/MetadataOp.java@496
PS3, Line 496:         upperCaseTableTypes.add(tableType);
I think the toUpperCase() here should stay.


http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/main/java/org/apache/impala/service/MetadataOp.java@497
PS3, Line 497:         if (tableType.equalsIgnoreCase(TABLE_TYPE_TABLE)) hasValidTableType
= true;
You can use equals() if you leave the toUpperCase() above.


http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/test/java/org/apache/impala/service/FrontendTest.java@181
PS3, Line 181:     // It should return two tables: one in "default"db and one in "testdb1".
space after "default"


http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/test/java/org/apache/impala/service/FrontendTest.java@209
PS3, Line 209:     assertEquals("alltypes_hive_view", resp.rows.get(0).colVals.get(2).string_val.toLowerCase());
Does this test succeed when run on its own? For these views to be returned they have to be
loaded, but that's not necessarily the case when running this test on its own.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90616388e6181cf342b3de389af940214ed46428
Gerrit-Change-Number: 7353
Gerrit-PatchSet: 3
Gerrit-Owner: sandeep akinapelli <sakinapelli@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mjacobs@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <vercegovac@cloudera.com>
Gerrit-Reviewer: sandeep akinapelli <sakinapelli@cloudera.com>
Gerrit-Comment-Date: Sat, 30 Sep 2017 03:13:31 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message