Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14799 )
Change subject: IMPALA-9101: Add support for detecting self-events on partition events ...................................................................... Patch Set 5: (3 comments) Looks good to me in general. Will give +1 after the comments are addressed. Thanks! http://gerrit.cloudera.org:8080/#/c/14799/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/14799/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2641 PS5, Line 2641: wasPartitionReloaded.setRef(true); It'd be better to move this line down after line 2643, or move line 2643 before this line. So the logic is more clear that whenever we bump table's catalog version, we set wasPartitionReloaded to true. (Same as line 2626-2629). http://gerrit.cloudera.org:8080/#/c/14799/5/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/14799/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@663 PS5, Line 663: refreshedTable.setCatalogVersion(newCatalogVersion); Could you comment about why we don't need addVersionsForInflightEvents here? http://gerrit.cloudera.org:8080/#/c/14799/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4307 PS5, Line 4307: loadTableMetadata(table, newCatalogVersion, true, false, partsToLoadMetadata, Should we add newCatalogVersion to the inflight versions of table and partitions? Could you add a test case for "insert into table xxx as select ..." to cover changes here? -- To view, visit http://gerrit.cloudera.org:8080/14799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b4148f6be0f9f946c8ad8f314d64b095731744c Gerrit-Change-Number: 14799 Gerrit-PatchSet: 5 Gerrit-Owner: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Thu, 09 Jan 2020 07:14:43 +0000 Gerrit-HasComments: Yes
