Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10171 )
Change subject: IMPALA-6916: Implement COMMENT ON DATABASE ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/10171/5/common/thrift/JniCatalog.thrift File common/thrift/JniCatalog.thrift: http://gerrit.cloudera.org:8080/#/c/10171/5/common/thrift/JniCatalog.thrift@634 PS5, Line 634: struct TCommentOnParams { > Is this enum needed? Can't we determine which comment we should update base Done http://gerrit.cloudera.org:8080/#/c/10171/5/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java File fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java: http://gerrit.cloudera.org:8080/#/c/10171/5/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java@30 PS5, Line 30: } > In Postgres you can remove a comment with: Done. I updated the grammar to allow NULL, too. http://gerrit.cloudera.org:8080/#/c/10171/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/10171/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3524 PS5, Line 3524: synchronized (metastoreDdlLock_) { > I don't think this is a precondition. The database could have legitimately Done. I'll throw an exception instead. http://gerrit.cloudera.org:8080/#/c/10171/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3528 PS5, Line 3528: addSummary(response, "Updated database."); > You need to increment and set the version of the db, not only the thrift re Done. Yeah I tested with multiple impalads. http://gerrit.cloudera.org:8080/#/c/10171/5/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: http://gerrit.cloudera.org:8080/#/c/10171/5/tests/metadata/test_ddl.py@197 PS5, Line 197: def test_comment_on_database(self, vector, unique_database): > test_comment_on_database Done -- To view, visit http://gerrit.cloudera.org:8080/10171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcf909c18f97073346f6f603538bf921e69fbb00 Gerrit-Change-Number: 10171 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Comment-Date: Fri, 27 Apr 2018 22:28:26 +0000 Gerrit-HasComments: Yes
