Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/13938 )
Change subject: IMPALA-8600: Refresh transactional tables ...................................................................... Patch Set 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/13938/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13938/6//COMMIT_MSG@7 PS6, Line 7: IMPALA-8600: Refresh transactional tables > nit: add : for sake of consistency with other commits Done http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3531 PS6, Line 3531: getTableFromMetaS > About the name: "get" does tell me whether the table is get from metastore I went for getTableFromMetaStore. Done http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3532 PS6, Line 3532: ame > If I see correctly tbl is only used in a precondition check. Indeed. Dropped. http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3626 PS6, Line 3626: throw new TableNotFoundException("Table not found: " + : tblName.toString()); > I am 100% sure, but it makes sense to me to throw the exception at line 367 Done http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3628 PS6, Line 3628: } : HdfsTable hdfsTable = (H > nit: indentation +4 Done http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3632 PS6, Line 3632: MetastoreShim.getWriteIdFromMSTable(hmsTbl)) { > Is this path tested? Even if there are "unneeded refreshes" among the ACID Done http://gerrit.cloudera.org:8080/#/c/13938/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3635 PS6, Line 3635: LOG.debug("Skip reloading table " + tblName.toString() + > It could be mentioned here that Hive seems to increase writeId for metadata That is the expected behaviour from Hive, I wouldn't comment it here. -- To view, visit http://gerrit.cloudera.org:8080/13938 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1851da22452074dbe253bcdd97145e06c7552cd3 Gerrit-Change-Number: 13938 Gerrit-PatchSet: 7 Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Wed, 31 Jul 2019 14:51:10 +0000 Gerrit-HasComments: Yes
