Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8315 )
Change subject: IMPALA-4918: Support getting column comments via HS2 ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/8315/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8315/2//COMMIT_MSG@9 PS2, Line 9: Fill the 'comments'/'remarks' field during HS2 column metadata requests. Please include in the commit message how you tested this. I think this also should include tests for other tables formats (Kudu, HBase). Those could be end to end tests. It would also be good to re-visit the existing "ALTER TABLE" tests and make sure they test adding comments. http://gerrit.cloudera.org:8080/#/c/8315/2/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java: http://gerrit.cloudera.org:8080/#/c/8315/2/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java@189 PS2, Line 189: dummyTable.addColumn(new Column(colDef.getColName(), colDef.getType(), colDef.getComment(), i)); Long line. http://gerrit.cloudera.org:8080/#/c/8315/2/fe/src/test/java/org/apache/impala/service/JdbcTest.java File fe/src/test/java/org/apache/impala/service/JdbcTest.java: http://gerrit.cloudera.org:8080/#/c/8315/2/fe/src/test/java/org/apache/impala/service/JdbcTest.java@460 PS2, Line 460: assertEquals("Incorrect table comment", "", rs.getString("REMARKS")); This looks like a bug in getTables() to me, but Alex and Dimitris would know better. -- To view, visit http://gerrit.cloudera.org:8080/8315 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d33dfd031b5344d7136695b623cec76143ada5c Gerrit-Change-Number: 8315 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Comment-Date: Wed, 18 Oct 2017 16:44:55 +0000 Gerrit-HasComments: Yes
