impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3724: Support Kudu non-covering range partitions
Date Fri, 28 Oct 2016 04:09:55 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-3724: Support Kudu non-covering range partitions
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4856/2/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:

Line 355:   2: optional ExternalDataSource.TComparisonOp lower_bound_op
> But this is where TComparisonOp leaves (which I agree is weird). We need to
Just to clarify: TComparisonOp lives in ExternalDataSource because we only need it for pushing
predicates to external data sources. The regular built-in exprs don't need a TComparisonOp
because we resolve the appropriate BE symbol in FE analysis, so the comparison op is baked
into that symbol.


Line 355:   2: optional ExternalDataSource.TComparisonOp lower_bound_op
> I take it back. Given the way it's been used in the catalog, one boolean ma
Let's do whatever feels cleanest. Lmk if you disagree with bool and we can still consider
alternatives.


-- 
To view, visit http://gerrit.cloudera.org:8080/4856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6799c01a37003f0f4c068d911a13e3f060110a06
Gerrit-PatchSet: 2
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