helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12977 )
Change subject: IMPALA-5351: Support storing column comment of kudu table ...................................................................... Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/12977/7/fe/src/main/java/org/apache/impala/catalog/KuduColumn.java File fe/src/main/java/org/apache/impala/catalog/KuduColumn.java: http://gerrit.cloudera.org:8080/#/c/12977/7/fe/src/main/java/org/apache/impala/catalog/KuduColumn.java@116 PS7, Line 116: > nit: remove two extra spaces Done http://gerrit.cloudera.org:8080/#/c/12977/7/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/12977/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2607 PS7, Line 2607: > nit: remove one extra space for the indentation Done http://gerrit.cloudera.org:8080/#/c/12977/7/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: http://gerrit.cloudera.org:8080/#/c/12977/7/tests/metadata/test_ddl.py@733 PS7, Line 733: create table {0} (i int PRIMARY KEY) STORED AS KUDU > nit: use lower case to be consistent with the style in this file Done http://gerrit.cloudera.org:8080/#/c/12977/7/tests/metadata/test_ddl.py@735 PS7, Line 735: comment = self._get_column_comment(table, 'i') > can we have similar test case for "create table {0} (x int comment 'x', pri Done http://gerrit.cloudera.org:8080/#/c/12977/7/tests/metadata/test_ddl_base.py File tests/metadata/test_ddl_base.py: http://gerrit.cloudera.org:8080/#/c/12977/7/tests/metadata/test_ddl_base.py@122 PS7, Line 122: or len(cols) == 9 > if len(cols) <= 9 is better since it will work on both kudu and on kudu tab Done -- To view, visit http://gerrit.cloudera.org:8080/12977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb3b37eed364f12bdb3c1d7ef5be128f1475936c Gerrit-Change-Number: 12977 Gerrit-PatchSet: 7 Gerrit-Owner: helifu <hzhel...@corp.netease.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: helifu <hzhel...@corp.netease.com> Gerrit-Comment-Date: Sun, 28 Apr 2019 01:35:20 +0000 Gerrit-HasComments: Yes
