Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/13938 )
Change subject: IMPALA-8600 Refresh transactional tables ...................................................................... Patch Set 5: (10 comments) http://gerrit.cloudera.org:8080/#/c/13938/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13938/4//COMMIT_MSG@20 PS4, Line 20: Similarl > nit: Similarly Done http://gerrit.cloudera.org:8080/#/c/13938/4//COMMIT_MSG@23 PS4, Line 23: changes like adding a column neither on table level or on partition > Offline you mentioned lastDdlTime. Maybe you can add a comment or TODO abou I have added a TODO to CatalogOpExecutor. I'll update that comment with Jira IDs. Is that ok? http://gerrit.cloudera.org:8080/#/c/13938/4//COMMIT_MSG@28 PS4, Line 28: updates the table level writeId even for schema changes. > You mentioned that it is a Hive-bug. Do we have an open Jira for that? Added it also to the TODO comment in CatalogOpExecutor. http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java: http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@161 PS4, Line 161: Refresh > nit: Refreshing a partition / REFRESH on a partition ? Done http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1900 PS4, Line 1900: > nit: Please add comment Done http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@733 PS4, Line 733: return true; > We imported it above. Done http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1370 PS4, Line 1370: > nit: on Done http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1473 PS4, Line 1473: > nit: on Done http://gerrit.cloudera.org:8080/#/c/13938/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1565 PS4, Line 1565: // fails, we throw a MetastoreNotificationNeedsInvalidateException : infoLog("{} partitions dropped from table {}. Trying " : + "to refresh.", droppedPartitions_.size(), getFullyQualifiedTblName()); : for (Map<String, String> partSpec : droppedPartitions_) { : List<TPartitionKeyValue> tPartSpec = new ArrayList<>(partSpec.size()); : for (Map.Entry<String, String> entry : partSpec.entrySet()) { : tPartSpec.add(new TPartitionKeyValue(entry.getKey(), entry.getValue())); : } : if (!catalog_.reloadPartitionIfExists(dbName_, tblName_, tPartSpec, : "processing DROP_PARTITION event from HMS")) { : de > nit: the above changes are quite similar. Can we put them into some helper Done http://gerrit.cloudera.org:8080/#/c/13938/4/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/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3611 PS4, Line 3611: isTableLoadedInCatalog > nit: I think this variable became unnecessary. It is necessary to capture if the table was loaded or not before we call getExistingTable() below. If it wasn't loaded then calling getExistingTable() would load it from HMS in the background and no need to do the refresh again. -- 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: 5 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 31 Jul 2019 09:55:03 +0000 Gerrit-HasComments: Yes
