You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/02/16 01:20:59 UTC

[impala] 09/11: IMPALA-6917: Implement COMMENT ON TABLE/VIEW

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 47f135dfcf6f0a157161ab4e9a0b2929abc3b0eb
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Tue May 22 16:40:47 2018 -0500

    IMPALA-6917: Implement COMMENT ON TABLE/VIEW
    
    This patch implements updating comment on a table or view.
    
    Syntax:
    COMMENT ON TABLE t IS 'comment'
    COMMENT ON VIEW v IS 'comment'
    
    Testing:
    - Added new front-end tests
    - Ran all front-end tests
    - Added new end-to-end tests
    - Ran end-to-end DDL tests
    
    Change-Id: I497c17342f79ff7c99931fd8a0ddec0c79303dbf
    Reviewed-on: http://gerrit.cloudera.org:8080/10478
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 common/thrift/JniCatalog.thrift                    |  9 +++-
 fe/src/main/cup/sql-parser.cup                     |  4 ++
 .../apache/impala/analysis/CommentOnDbStmt.java    |  2 +
 .../org/apache/impala/analysis/CommentOnStmt.java  | 13 +++++-
 ...OnDbStmt.java => CommentOnTableOrViewStmt.java} | 35 +++++++++++----
 ...{CommentOnStmt.java => CommentOnTableStmt.java} | 22 ++++-----
 .../{CommentOnStmt.java => CommentOnViewStmt.java} | 22 ++++-----
 .../apache/impala/service/CatalogOpExecutor.java   | 37 ++++++++++++++-
 .../org/apache/impala/analysis/AnalyzeDDLTest.java | 52 +++++++++++++++++++++-
 .../apache/impala/analysis/AuthorizationTest.java  | 16 +++++++
 .../org/apache/impala/analysis/ParserTest.java     | 15 +++++++
 tests/metadata/test_ddl.py                         | 50 +++++++++++++++++++++
 tests/metadata/test_ddl_base.py                    |  6 ++-
 13 files changed, 248 insertions(+), 35 deletions(-)

diff --git a/common/thrift/JniCatalog.thrift b/common/thrift/JniCatalog.thrift
index 87722a4..7052faf 100644
--- a/common/thrift/JniCatalog.thrift
+++ b/common/thrift/JniCatalog.thrift
@@ -663,9 +663,16 @@ struct TGetCatalogUsageResponse{
 }
 
 struct TCommentOnParams {
-  // Name of comment to alter. When this field is not set, the comment will be removed.
+  // Contents of comment to alter. When this field is not set, the comment will be removed.
   1: optional string comment
 
+  //--------------------------------------
+  // Only one of these fields can be set.
+  //--------------------------------------
+
   // Name of database to alter.
   2: optional string db
+
+  // Name of table/view to alter.
+  3: optional CatalogObjects.TTableName table_name
 }
diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
index 352f185..db69aae 100644
--- a/fe/src/main/cup/sql-parser.cup
+++ b/fe/src/main/cup/sql-parser.cup
@@ -1030,6 +1030,10 @@ partition_def_list ::=
 comment_on_stmt ::=
   KW_COMMENT KW_ON KW_DATABASE ident_or_default:db_name KW_IS nullable_comment_val:comment
   {: RESULT = new CommentOnDbStmt(db_name, comment); :}
+  | KW_COMMENT KW_ON KW_TABLE table_name:table KW_IS nullable_comment_val:comment
+  {: RESULT = new CommentOnTableStmt(table, comment); :}
+  | KW_COMMENT KW_ON KW_VIEW table_name:table KW_IS nullable_comment_val:comment
+  {: RESULT = new CommentOnViewStmt(table, comment); :}
   ;
 
 // Introducing OWNER and USER keywords has a potential to be a breaking change,
diff --git a/fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java b/fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java
index 8f622fd..a216fd3 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java
@@ -36,9 +36,11 @@ public class CommentOnDbStmt extends CommentOnStmt {
 
   @Override
   public void analyze(Analyzer analyzer) throws AnalysisException {
+    super.analyze(analyzer);
     analyzer.getDb(dbName_, Privilege.ALTER);
   }
 
+  @Override
   public TCommentOnParams toThrift() {
     TCommentOnParams params = super.toThrift();
     params.setDb(dbName_);
diff --git a/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java b/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
index dffc08d..667659a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
@@ -17,18 +17,29 @@
 
 package org.apache.impala.analysis;
 
+import org.apache.impala.common.AnalysisException;
 import org.apache.impala.thrift.TCommentOnParams;
+import org.apache.impala.util.MetaStoreUtil;
 
 /**
  * A base class for COMMENT ON statement.
  */
-public class CommentOnStmt extends StatementBase {
+public abstract class CommentOnStmt extends StatementBase {
   protected final String comment_;
 
   public CommentOnStmt(String comment) {
     comment_ = comment;
   }
 
+  @Override
+  public void analyze(Analyzer analyzer) throws AnalysisException {
+    if (comment_ != null && comment_.length() > MetaStoreUtil.CREATE_MAX_COMMENT_LENGTH) {
+      throw new AnalysisException(String.format("Comment exceeds maximum length of %d " +
+          "characters. The given comment has %d characters.",
+          MetaStoreUtil.CREATE_MAX_COMMENT_LENGTH, comment_.length()));
+    }
+  }
+
   public TCommentOnParams toThrift() {
     TCommentOnParams params = new TCommentOnParams();
     params.setComment(comment_);
diff --git a/fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java b/fe/src/main/java/org/apache/impala/analysis/CommentOnTableOrViewStmt.java
similarity index 55%
copy from fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java
copy to fe/src/main/java/org/apache/impala/analysis/CommentOnTableOrViewStmt.java
index 8f622fd..36325a6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CommentOnDbStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CommentOnTableOrViewStmt.java
@@ -22,26 +22,45 @@ import org.apache.impala.authorization.Privilege;
 import org.apache.impala.common.AnalysisException;
 import org.apache.impala.thrift.TCommentOnParams;
 
+import java.util.List;
+
 /**
- * Represents a COMMENT ON DATABASE db IS 'comment' statement.
+ * A base class for COMMENT ON TABLE/VIEW.
  */
-public class CommentOnDbStmt extends CommentOnStmt {
-  private final String dbName_;
+public abstract class CommentOnTableOrViewStmt extends CommentOnStmt {
+  protected TableName tableName_;
 
-  public CommentOnDbStmt(String dbName, String comment) {
+  public CommentOnTableOrViewStmt(TableName tableName, String comment) {
     super(comment);
-    Preconditions.checkNotNull(dbName);
-    dbName_ = dbName;
+    Preconditions.checkArgument(tableName != null && !tableName.isEmpty());
+    tableName_ = tableName;
+  }
+
+  @Override
+  public void collectTableRefs(List<TableRef> tblRefs) {
+    tblRefs.add(new TableRef(tableName_.toPath(), null));
   }
 
   @Override
   public void analyze(Analyzer analyzer) throws AnalysisException {
-    analyzer.getDb(dbName_, Privilege.ALTER);
+    super.analyze(analyzer);
+    tableName_ = analyzer.getFqTableName(tableName_);
+    TableRef tableRef = new TableRef(tableName_.toPath(), null, Privilege.ALTER);
+    tableRef = analyzer.resolveTableRef(tableRef);
+    Preconditions.checkNotNull(tableRef);
+    tableRef.analyze(analyzer);
+    validateType(tableRef);
   }
 
+  /**
+   * Validates the type of the given TableRef.
+   */
+  protected abstract void validateType(TableRef tableRef) throws AnalysisException;
+
+  @Override
   public TCommentOnParams toThrift() {
     TCommentOnParams params = super.toThrift();
-    params.setDb(dbName_);
+    params.setTable_name(tableName_.toThrift());
     return params;
   }
 }
diff --git a/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java b/fe/src/main/java/org/apache/impala/analysis/CommentOnTableStmt.java
similarity index 61%
copy from fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
copy to fe/src/main/java/org/apache/impala/analysis/CommentOnTableStmt.java
index dffc08d..d7daa40 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CommentOnTableStmt.java
@@ -17,21 +17,21 @@
 
 package org.apache.impala.analysis;
 
-import org.apache.impala.thrift.TCommentOnParams;
+import org.apache.impala.common.AnalysisException;
 
 /**
- * A base class for COMMENT ON statement.
+ * Represents a COMMENT ON TABLE tbl IS 'comment' statement.
  */
-public class CommentOnStmt extends StatementBase {
-  protected final String comment_;
-
-  public CommentOnStmt(String comment) {
-    comment_ = comment;
+public class CommentOnTableStmt extends CommentOnTableOrViewStmt {
+  public CommentOnTableStmt(TableName tableName, String comment) {
+    super(tableName, comment);
   }
 
-  public TCommentOnParams toThrift() {
-    TCommentOnParams params = new TCommentOnParams();
-    params.setComment(comment_);
-    return params;
+  @Override
+  protected void validateType(TableRef tableRef) throws AnalysisException {
+    if (tableRef instanceof InlineViewRef) {
+      throw new AnalysisException(String.format(
+          "COMMENT ON TABLE not allowed on a view: %s", tableName_));
+    }
   }
 }
diff --git a/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java b/fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java
similarity index 61%
copy from fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
copy to fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java
index dffc08d..cbd3f6b 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CommentOnStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CommentOnViewStmt.java
@@ -17,21 +17,21 @@
 
 package org.apache.impala.analysis;
 
-import org.apache.impala.thrift.TCommentOnParams;
+import org.apache.impala.common.AnalysisException;
 
 /**
- * A base class for COMMENT ON statement.
+ * Represents a COMMENT ON VIEW v IS 'comment' statement.
  */
-public class CommentOnStmt extends StatementBase {
-  protected final String comment_;
-
-  public CommentOnStmt(String comment) {
-    comment_ = comment;
+public class CommentOnViewStmt extends CommentOnTableOrViewStmt {
+  public CommentOnViewStmt(TableName tableName, String comment) {
+    super(tableName, comment);
   }
 
-  public TCommentOnParams toThrift() {
-    TCommentOnParams params = new TCommentOnParams();
-    params.setComment(comment_);
-    return params;
+  @Override
+  protected void validateType(TableRef tableRef) throws AnalysisException {
+    if (!(tableRef instanceof InlineViewRef)) {
+      throw new AnalysisException(String.format(
+          "COMMENT ON VIEW not allowed on a table: %s", tableName_));
+    }
   }
 }
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 2d83bab..f846e60 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -3502,12 +3502,19 @@ public class CatalogOpExecutor {
   private void alterCommentOn(TCommentOnParams params, TDdlExecResponse response)
       throws ImpalaRuntimeException, CatalogException, InternalException {
     if (params.getDb() != null) {
+      Preconditions.checkArgument(!params.isSetTable_name());
       alterCommentOnDb(params.getDb(), params.getComment(), response);
+    } else if (params.getTable_name() != null) {
+      Preconditions.checkArgument(!params.isSetDb());
+      alterCommentOnTableOrView(TableName.fromThrift(params.getTable_name()),
+          params.getComment(), response);
+    } else {
+      throw new UnsupportedOperationException("Unsupported COMMENT ON operation");
     }
   }
 
   private void alterCommentOnDb(String dbName, String comment, TDdlExecResponse response)
-      throws ImpalaRuntimeException, CatalogException, InternalException {
+      throws ImpalaRuntimeException, CatalogException {
     Db db = catalog_.getDb(dbName);
     if (db == null) {
       throw new CatalogException("Database: " + db.getName() + " does not exist.");
@@ -3582,4 +3589,32 @@ public class CatalogOpExecutor {
       catalog_.getLock().writeLock().unlock();
     }
   }
+
+  private void alterCommentOnTableOrView(TableName tableName, String comment,
+      TDdlExecResponse response) throws CatalogException, InternalException,
+      ImpalaRuntimeException {
+    Table tbl = getExistingTable(tableName.getDb(), tableName.getTbl());
+    if (!catalog_.tryLockTable(tbl)) {
+      throw new InternalException(String.format("Error altering table/view %s due to " +
+          "lock contention.", tbl.getFullName()));
+    }
+    try {
+      long newCatalogVersion = catalog_.incrementAndGetCatalogVersion();
+      catalog_.getLock().writeLock().unlock();
+      org.apache.hadoop.hive.metastore.api.Table msTbl = tbl.getMetaStoreTable().deepCopy();
+      boolean isView = msTbl.getTableType().equalsIgnoreCase(
+          TableType.VIRTUAL_VIEW.toString());
+      if (comment == null) {
+        msTbl.getParameters().remove("comment");
+      } else {
+        msTbl.getParameters().put("comment", comment);
+      }
+      applyAlterTable(msTbl, true);
+      loadTableMetadata(tbl, newCatalogVersion, false, false, null);
+      addTableToCatalogUpdate(tbl, response.result);
+      addSummary(response, String.format("Updated %s.", (isView) ? "view" : "table"));
+    } finally {
+      tbl.getLock().unlock();
+    }
+  }
 }
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
index 5ab533c..634fb87 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -46,6 +46,7 @@ import org.apache.impala.catalog.Type;
 import org.apache.impala.common.AnalysisException;
 import org.apache.impala.common.FileSystemUtil;
 import org.apache.impala.common.FrontendTestBase;
+import org.apache.impala.common.Pair;
 import org.apache.impala.common.RuntimeEnv;
 import org.apache.impala.service.BackendConfig;
 import org.apache.impala.testutil.TestUtils;
@@ -3912,12 +3913,61 @@ public class AnalyzeDDLTest extends FrontendTestBase {
   }
 
   @Test
-  public void TestCommentOn() {
+  public void TestCommentOnDatabase() {
     AnalyzesOk("comment on database functional is 'comment'");
     AnalyzesOk("comment on database functional is ''");
     AnalyzesOk("comment on database functional is null");
     AnalysisError("comment on database doesntexist is 'comment'",
         "Database does not exist: doesntexist");
+    AnalysisError(String.format("comment on database functional is '%s'",
+        buildLongComment()), "Comment exceeds maximum length of 256 characters. " +
+        "The given comment has 261 characters.");
+  }
+
+  @Test
+  public void TestCommentOnTable() {
+    for (Pair<String, AnalysisContext> pair : new Pair[]{
+        new Pair<>("functional.alltypes", createAnalysisCtx()),
+        new Pair<>("alltypes", createAnalysisCtx("functional"))}) {
+      AnalyzesOk(String.format("comment on table %s is 'comment'", pair.first),
+          pair.second);
+      AnalyzesOk(String.format("comment on table %s is ''", pair.first), pair.second);
+      AnalyzesOk(String.format("comment on table %s is null", pair.first), pair.second);
+    }
+    AnalysisError("comment on table doesntexist is 'comment'",
+        "Could not resolve table reference: 'default.doesntexist'");
+    AnalysisError("comment on table functional.alltypes_view is 'comment'",
+        "COMMENT ON TABLE not allowed on a view: functional.alltypes_view");
+    AnalysisError(String.format("comment on table functional.alltypes is '%s'",
+        buildLongComment()), "Comment exceeds maximum length of 256 characters. " +
+        "The given comment has 261 characters.");
+  }
+
+  @Test
+  public void TestCommentOnView() {
+    for (Pair<String, AnalysisContext> pair : new Pair[]{
+        new Pair<>("functional.alltypes_view", createAnalysisCtx()),
+        new Pair<>("alltypes_view", createAnalysisCtx("functional"))}) {
+      AnalyzesOk(String.format("comment on view %s is 'comment'", pair.first),
+          pair.second);
+      AnalyzesOk(String.format("comment on view %s is ''", pair.first), pair.second);
+      AnalyzesOk(String.format("comment on view %s is null", pair.first), pair.second);
+    }
+    AnalysisError("comment on view doesntexist is 'comment'",
+        "Could not resolve table reference: 'default.doesntexist'");
+    AnalysisError("comment on view functional.alltypes is 'comment'",
+        "COMMENT ON VIEW not allowed on a table: functional.alltypes");
+    AnalysisError(String.format("comment on table functional.alltypes_view is '%s'",
+        buildLongComment()), "Comment exceeds maximum length of 256 characters. " +
+        "The given comment has 261 characters.");
+  }
+
+  private static String buildLongComment() {
+    StringBuilder comment = new StringBuilder();
+    for (int i = 0; i < MetaStoreUtil.CREATE_MAX_COMMENT_LENGTH + 5; i++) {
+      comment.append("a");
+    }
+    return comment.toString();
   }
 
   @Test
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
index ed6f0e4..36bcaa3 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -3015,6 +3015,22 @@ public class AuthorizationTest extends FrontendTestBase {
         "User '%s' does not have privileges to execute 'ALTER' on: functional");
     AuthzError("comment on database doesntexist is 'comment'",
         "User '%s' does not have privileges to execute 'ALTER' on: doesntexist");
+
+    // User has ALTER privilege on functional_text_lzo.alltypes table.
+    AuthzOk("comment on table functional_text_lzo.alltypes is 'comment'");
+    // User does not have ALTER privilege on functional.alltypes table.
+    AuthzError("comment on table functional.alltypes is 'comment'",
+        "User '%s' does not have privileges to execute 'ALTER' on: functional");
+    AuthzError("comment on table doesntexist is 'comment'",
+        "User '%s' does not have privileges to execute 'ALTER' on: default.doesntexist");
+
+    // User has ALTER privilege on functional.alltypes_view view.
+    AuthzOk("comment on view functional.alltypes_view is 'comment'");
+    // User does not have ALTER privilege on functional.view_view table.
+    AuthzError("comment on view functional.view_view is 'comment'",
+        "User '%s' does not have privileges to execute 'ALTER' on: functional");
+    AuthzError("comment on view doesntexist is 'comment'",
+        "User '%s' does not have privileges to execute 'ALTER' on: default.doesntexist");
   }
 
   @Test
diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
index ba01564..93c2e2b 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
@@ -3746,6 +3746,21 @@ public class ParserTest extends FrontendTestBase {
     ParsesOk("COMMENT ON DATABASE db IS NULL");
     ParserError("COMMENT ON DATABASE IS 'comment'");
     ParserError("COMMENT ON DATABASE db IS");
+
+    for (String tbl : new String[]{"db.t", "t"}) {
+      ParsesOk(String.format("COMMENT ON TABLE %s IS 'comment'", tbl));
+      ParsesOk(String.format("COMMENT ON TABLE %s IS ''", tbl));
+      ParsesOk(String.format("COMMENT ON TABLE %s IS NULL", tbl));
+
+      ParsesOk(String.format("COMMENT ON VIEW %s IS 'comment'", tbl));
+      ParsesOk(String.format("COMMENT ON VIEW %s IS ''", tbl));
+      ParsesOk(String.format("COMMENT ON VIEW %s IS NULL", tbl));
+    }
+    ParserError("COMMENT ON TABLE IS 'comment'");
+    ParserError("COMMENT ON TABLE tbl IS");
+
+    ParserError("COMMENT ON VIEW IS 'comment'");
+    ParserError("COMMENT ON VIEW tbl IS");
   }
 
   @Test
diff --git a/tests/metadata/test_ddl.py b/tests/metadata/test_ddl.py
index e11fb09..bcafd8e 100644
--- a/tests/metadata/test_ddl.py
+++ b/tests/metadata/test_ddl.py
@@ -202,6 +202,10 @@ class TestDdlStatements(TestDdlBase):
     comment = self._get_db_comment(unique_database)
     assert 'comment' == comment
 
+    self.client.execute("comment on database {0} is '\\'comment\\''".format(unique_database))
+    comment = self._get_db_comment(unique_database)
+    assert "\\'comment\\'" == comment
+
     self.client.execute("comment on database {0} is ''".format(unique_database))
     comment = self._get_db_comment(unique_database)
     assert '' == comment
@@ -258,6 +262,52 @@ class TestDdlStatements(TestDdlBase):
     self.run_test_case('QueryTest/kudu_create', vector, use_db=unique_database,
         multiple_impalad=self._use_multiple_impalad(vector))
 
+  def test_comment_on_table(self, vector, unique_database):
+    table = '{0}.comment_table'.format(unique_database)
+    self.client.execute("create table {0} (i int)".format(table))
+
+    comment = self._get_table_or_view_comment(table)
+    assert comment is None
+
+    self.client.execute("comment on table {0} is 'comment'".format(table))
+    comment = self._get_table_or_view_comment(table)
+    assert "comment" == comment
+
+    self.client.execute("comment on table {0} is '\\'comment\\''".format(table))
+    comment = self._get_table_or_view_comment(table)
+    assert "\\\\'comment\\\\'" == comment
+
+    self.client.execute("comment on table {0} is ''".format(table))
+    comment = self._get_table_or_view_comment(table)
+    assert "" == comment
+
+    self.client.execute("comment on table {0} is null".format(table))
+    comment = self._get_table_or_view_comment(table)
+    assert comment is None
+
+  def test_comment_on_view(self, vector, unique_database):
+    view = '{0}.comment_view'.format(unique_database)
+    self.client.execute("create view {0} as select 1".format(view))
+
+    comment = self._get_table_or_view_comment(view)
+    assert comment is None
+
+    self.client.execute("comment on view {0} is 'comment'".format(view))
+    comment = self._get_table_or_view_comment(view)
+    assert "comment" == comment
+
+    self.client.execute("comment on view {0} is '\\'comment\\''".format(view))
+    comment = self._get_table_or_view_comment(view)
+    assert "\\\\'comment\\\\'" == comment
+
+    self.client.execute("comment on view {0} is ''".format(view))
+    comment = self._get_table_or_view_comment(view)
+    assert "" == comment
+
+    self.client.execute("comment on view {0} is null".format(view))
+    comment = self._get_table_or_view_comment(view)
+    assert comment is None
+
   @UniqueDatabase.parametrize(sync_ddl=True)
   def test_sync_ddl_drop(self, vector, unique_database):
     """Verifies the catalog gets updated properly when dropping objects with sync_ddl
diff --git a/tests/metadata/test_ddl_base.py b/tests/metadata/test_ddl_base.py
index cc0e0c2..c1388fc 100644
--- a/tests/metadata/test_ddl_base.py
+++ b/tests/metadata/test_ddl_base.py
@@ -96,4 +96,8 @@ class TestDdlBase(ImpalaTestSuite):
   def _get_db_comment(self, db_name):
     """Extracts the DB comment from the output of DESCRIBE DATABASE"""
     result = self.client.execute("describe database {0}".format(db_name))
-    return result.data[0].split('\t')[2]
\ No newline at end of file
+    return result.data[0].split('\t')[2]
+
+  def _get_table_or_view_comment(self, table_name):
+    props = self._get_tbl_properties(table_name)
+    return props["comment"] if "comment" in props else None
\ No newline at end of file