Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables ......................................................................
Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/4414/4/fe/src/main/java/org/apache/impala/analysis/DistributeParam.java File fe/src/main/java/org/apache/impala/analysis/DistributeParam.java: Line 129: if (!org.apache.impala.catalog.Type.areAssignmentCompatibleTypes(colType, > Done you want to be able to assign the split value to the column type with just an implicit cast, if any. in other words, for a bigint col it's okay to have a split value of 127, but for a tinyint col, you can't have a split value of maxint. you can call isImplicitlyCastable(exprType, colType). http://gerrit.cloudera.org:8080/#/c/4414/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: PS4, Line 226: private void loadDistributeByParams(org.apache.kudu.client.KuduTable kuduTable) { : Preconditions > cols is a reference to msTable cols. We clear them here and reload them fro yes, please leave a comment that you're resetting the mstable cols if there are any. i tripped over that as well. http://gerrit.cloudera.org:8080/#/c/4414/5/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: Line 157: public void load(boolean reuseMetadata /* not used */, IMetaStoreClient msClient, rename reuseMetadata to dummy Line 217: cols.add(new FieldSchema(colName, type.toSql().toLowerCase(), null)); this also updates msTable_ document this side effect in the function comment. this feels pretty convoluted and is hard to follow. is this really necessary or should we simply ignore the column info stored in mstable? Line 295: TDistributeByRangeParam rangeParam = param.getBy_range_param(); checkstate(isset...) -- To view, visit http://gerrit.cloudera.org:8080/4414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b9d51b2720ab57649abdb7d5c710ea04ff50dc1 Gerrit-PatchSet: 5 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-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-HasComments: Yes
