Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15260 )
Change subject: IMPALA-9357: Fix race condition in alter_database event ...................................................................... Patch Set 8: Code-Review+1 (3 comments) lgtm, just some nits http://gerrit.cloudera.org:8080/#/c/15260/8/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/15260/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@222 PS8, Line 222: CREATE/DROP : * function nit: This was a bit ambigious at first, as it could mean any function that creates/drops. FUNCTION could be also capitalized to highlight that is is an SQL keyword, not a java function. http://gerrit.cloudera.org:8080/#/c/15260/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@224 PS8, Line 224: , nit: , not needed http://gerrit.cloudera.org:8080/#/c/15260/8/tests/custom_cluster/test_concurrent_ddls.py File tests/custom_cluster/test_concurrent_ddls.py: http://gerrit.cloudera.org:8080/#/c/15260/8/tests/custom_cluster/test_concurrent_ddls.py@108 PS8, Line 108: "symbol='NoArgs'" % (test_db, WAREHOUSE), nit: can you add 4 indents to highlight that this is the continuation of the last line? -- To view, visit http://gerrit.cloudera.org:8080/15260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e Gerrit-Change-Number: 15260 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Tue, 10 Mar 2020 09:24:29 +0000 Gerrit-HasComments: Yes
