Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3726: Add support for Kudu-specific column options ......................................................................
Patch Set 7: Code-Review+2 (2 comments) Rebase and fixing tests, carry Alex's +2 http://gerrit.cloudera.org:8080/#/c/5026/5/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java File fe/src/main/java/org/apache/impala/analysis/ColumnDef.java: Line 129: public ColumnDef(String colName, TypeDef typeDef) { > Seems like we'd often need a C'tor that also takes a comment string. You ca Not as simple as it sounds, we need to make sure we don't insert nulls and then populate the map. But because the call to the delegating constructor needs to be the first statement that needs to happen outside the constructor. http://gerrit.cloudera.org:8080/#/c/5026/5/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: Line 250: def test_primary_key_and_distribution(self, cursor): > AUTO and DEFAULT still seem uninformative. Let's revisit later like you sug I agree for encoding, not so much for compression since it can have no_compression. Anyways, as you said, let's revisit this later. -- To view, visit http://gerrit.cloudera.org:8080/5026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I727b9ae1b7b2387db752b58081398dd3f3449c02 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-HasComments: Yes
