Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables

Patch Set 5:

File fe/src/main/java/org/apache/impala/analysis/

Line 129:           if 
> 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 

you can call isImplicitlyCastable(exprType, colType).
File fe/src/main/java/org/apache/impala/catalog/

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.
File fe/src/main/java/org/apache/impala/catalog/

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(), 
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 = 

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b9d51b2720ab57649abdb7d5c710ea04ff50dc1
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-HasComments: Yes

Reply via email to