Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10478 )
Change subject: IMPALA-6917: Implement COMMENT ON TABLE/VIEW ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/10478/4/common/thrift/JniCatalog.thrift File common/thrift/JniCatalog.thrift: http://gerrit.cloudera.org:8080/#/c/10478/4/common/thrift/JniCatalog.thrift@635 PS4, Line 635: Contents of comment to a > I realize its from a prev. change, but what does "Name of comment" mean? Pe Done http://gerrit.cloudera.org:8080/#/c/10478/4/common/thrift/JniCatalog.thrift@638 PS4, Line 638: //-------------------------------------- : // Only one of these fields can be set. : //-------------------------------------- : : // Name of database to alter. > state that exactly one of these must be set. its ambiguous if comment can b Done http://gerrit.cloudera.org:8080/#/c/10478/4/fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java File fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java: http://gerrit.cloudera.org:8080/#/c/10478/4/fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java@39 PS4, Line 39: : : : > It looks like this is the key thing that differentiates view vs. table. How Done http://gerrit.cloudera.org:8080/#/c/10478/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/10478/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@3915 PS4, Line 3915: buildLongComment()), "Comment exceeds maximum length of 256 characters. " + : "The given comment has 261 characters."); : } > perhaps factor this if it does not obfuscate the test. Done http://gerrit.cloudera.org:8080/#/c/10478/4/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: http://gerrit.cloudera.org:8080/#/c/10478/4/tests/metadata/test_ddl.py@255 PS4, Line 255: > perhaps try: Done. I updated the code and some unit tests to check for the limit. -- To view, visit http://gerrit.cloudera.org:8080/10478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I497c17342f79ff7c99931fd8a0ddec0c79303dbf Gerrit-Change-Number: 10478 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Sat, 02 Jun 2018 00:33:51 +0000 Gerrit-HasComments: Yes
