Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14296 )
Change subject: IMPALA-8968: Alter database events on dropped database should not put events processor in error state. ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/14296/2/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/14296/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2220 PS2, Line 2220: > Any reason this is CatalogException instead of DatabaseNotFound...? Done http://gerrit.cloudera.org:8080/#/c/14296/2/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/14296/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1228 PS2, Line 1228: ugLo > nit: Okay Done http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1235 PS2, Line 1235: > Consider moving the exception handling to updateDbIfExists? Done http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@557 PS2, Line 557: scalar function > Do you think we need to add coverage for native functions here? There's a i This change is adding the catalog service identifiers to the DB irrespective of weather it is a persistent java function or not. In case of persistent java functions, hms creates a CREATE_FUNCTION event which we do not support. We ignore such events anyway. http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@580 PS2, Line 580: eventsProcessor_.processEvents(); > Need to call eventsProcessor_.processEvents() before this? Good catch. Done. http://gerrit.cloudera.org:8080/#/c/14296/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2336 PS2, Line 2336: > Typo, impala Oops! -- To view, visit http://gerrit.cloudera.org:8080/14296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f76136aeff35d1d38fc8b3d9a38da399d36eced Gerrit-Change-Number: 14296 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada <anu...@cloudera.com> Gerrit-Reviewer: Anurag Mantripragada <anu...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Thu, 26 Sep 2019 18:54:27 +0000 Gerrit-HasComments: Yes
