Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13874 )

Change subject: IMPALA-8606: Don't load table meta for GET_TABLES in local 
catalog mode
......................................................................


Patch Set 6:

(4 comments)

Addressing the comments. Trying to test it in a live cluster with HUE.

http://gerrit.cloudera.org:8080/#/c/13874/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13874/5//COMMIT_MSG@23
PS5, Line 23:  - Testing in a HMS with 100 dbs and 3000 tables, without this 
patch it
> Mind deploying this jar on a cluster with Hue and quickly check that nothin
Sure. Will do this.


http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/Catalog.java
File fe/src/main/java/org/apache/impala/catalog/Catalog.java:

http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/Catalog.java@167
PS5, Line 167: Catalog v1
> I think the code never refers to Catalog "v1" and Catalog "v2". So this cou
I'm trying to explain why the implementation here is just delegating to 
getTableNoThrow. This function will only be called by impalad. Code path came 
here when in Catalog v1. Otherwise, the code path will go to LocalCatalog's 
implementation.

What about putting such kind of comments inside the function? Will it be less 
confusing?


http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java:

http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java@169
PS5, Line 169:     } catch (TException e) {
> Should we instead load LocalIncompleteTable here?
Cool! Done.


http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:

http://gerrit.cloudera.org:8080/#/c/13874/5/fe/src/test/java/org/apache/impala/service/JdbcTest.java@390
PS5, Line 390:   public void testMetaDataGetColumnComments() throws Exception {
> Mind adding a test to LocalCatalogTest#testGetTables()? Check that the tabl
Sure



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8bbab7efdf8e629abe09d89ae3bd770e3feaccb
Gerrit-Change-Number: 13874
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 23:23:18 +0000
Gerrit-HasComments: Yes

Reply via email to