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)

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 : "");
             :         }
             :       }
> Regarding your suggestion, there was no loaded tables when I attached debug
The catalog server is the only entity responsible for loading metadata and 
making calls to HMS (for that purpose). On the other hand, Impalad's catalog 
cache should not be doing anything like that (which is what you do in this 
block). JniFronted::getTableNames() retrieves and returns the names of tables 
that the local catalog cache knows of (no external communication with either 
the catalog server or the HMS).

So, what you should do is the following: If the table is fully loaded and has a 
comment, you can retrieve it directly from the Table object. If the table is 
not fully loaded, you don't do anything (return empty string)  even if the 
table has a comment stored in HMS. If the table is loaded but has no comment, 
return an empty string.

If the concepts of fully loaded metadata, catalog server and impalad catalog 
cache are not clear, I suggest you spend some time at the code before working 
on this change. Impala's metadata management system is a bit complex, so it 
takes some time to grasp all the details and the information flow.



--
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: Mon, 08 Jan 2018 18:34:05 +0000
Gerrit-HasComments: Yes

Reply via email to