Dimitris Tsirogiannis has posted comments on this change. ( http://gerrit.cloudera.org:8080/8851 )
Change subject: IMPALA-3193: Show table's comment on show tables ...................................................................... Patch Set 4: (1 comment) Thanks for looking into it. I just wanted to iterate that it's a good practice to respond to high level comments so that reviewers have a better understanding of what kind of changes are in the patch that they are reviewing. In most cases, a simple "DONE" is sufficient, but you can use your judgement to see what kind of response is needed depending on the ask. http://gerrit.cloudera.org:8080/#/c/8851/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/8851/4/fe/src/main/java/org/apache/impala/service/JniFrontend.java@304 PS4, Line 304: for (Table table: tables) { : org.apache.hadoop.hive.metastore.api.Table msTable : = table.getMetaStoreTable(msClient); : if (msTable != null) { : String comment = msTable.getParameters().get("comment"); : comments.add(comment != null ? comment : ""); : } : } This is not acceptable from a performance point of view. You're making a call to HMS for every table that satisfies the params.pattern just to get the comment. You should return the comment (if any) only for already loaded tables (i.e. no table loading and no external calls to either the catalog or the HMS). -- To view, visit http://gerrit.cloudera.org:8080/8851 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd Gerrit-Change-Number: 8851 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Comment-Date: Fri, 05 Jan 2018 22:44:24 +0000 Gerrit-HasComments: Yes
