impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3726: Add support for Kudu-specific column options
Date Thu, 10 Nov 2016 07:22:14 GMT
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-3726: Add support for Kudu-specific column options
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5026/1/fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
File fe/src/main/java/org/apache/impala/analysis/ColumnDef.java:

Line 255:         defaultValue_ = LiteralExpr.create(castLiteral, analyzer.getQueryCtx());
> I remember that earlier case (maybe we got it wrong there, too?).
That's not a problem because both tinyint and int are stored in thrift as i64. When the catalog
sets the value in the Kudu PartialRow it will cast the stored value to the appropriate primitive
type based on the Kudu type.  The problem was that if the column is int and the value is boolean,
the latter is stored in a bool in thrift but the catalog expects to find an IntLiteral. So,
the error was thrown from the catalog for a case that should have worked (i.e. bool ->
int). It may be an overkill but it seems to be the cleanest way to handle different types
in default values and the associated columns.


-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message