Kim Jin Chul 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 15:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8851/15/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/8851/15/fe/src/main/java/org/apache/impala/service/MetadataOp.java@279
PS15, Line 279: for (Table table: fe.getTables(db.getName(), 
tablePatternMatcher, user)) {
              :           String tabName = table.getName();
> Are we certain that this code doesn't introduce the same infinite wait for
I confirmed the problem should be solved with my change. The cause of the 
problem is JUnit test requires tables loading explicitly if they are not 
loaded. This is the reason why I introduce a new code at 
ImpaladTestCatalog.java.


http://gerrit.cloudera.org:8080/#/c/8851/15/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
File fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java:

http://gerrit.cloudera.org:8080/#/c/8851/15/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java@107
PS15, Line 107: /**
              :    * Overrides ImpaladCatalog.getTables to load the table 
metadata if it is missing.
              :    */
              :   @Override
              :   public List<Table> getTables(String dbName, PatternMatcher 
matcher)
              :       throws DatabaseNotFoundException {
              :     List<Table> tables = super.getTables(dbName, matcher);
              :
              :     // The table was not yet loaded. Load it in to the catalog 
and try getTable()
              :     // again.
              :     for (int i = 0; i < tables.size(); ++i) {
              :       Table table = tables.get(i);
              :       // Table doesn't exist or is already loaded. Just return 
it.
              :       if (table != null && !table.isLoaded()) {
              :         try {
              :           tables.set(i, getLoadedTable(dbName, 
table.getName()));
              :         } catch (CatalogException e) {
              :           LOG.error(String.format("Error loading table: %s.%s", 
dbName, table.getName()),
              :               e);
              :         }
              :       }
              :     }
              :     return tables;
              :   }
> Where is the ImpaladTestCatalog::getTables() called?
During JUnit test, it should be called at 
"impaladCatalog_.get().getTables(dbName, matcher)". See 
https://gerrit.cloudera.org/#/c/8851/15/fe/src/main/java/org/apache/impala/service/Frontend.java
 at 625.



--
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: 15
Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Feb 2018 23:17:02 +0000
Gerrit-HasComments: Yes

Reply via email to