impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4192: Disentangle Expr and ExprContext
Date Wed, 25 Jan 2017 16:17:27 GMT
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4192: Disentangle Expr and ExprContext
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5483/6/be/src/exprs/expr.h
File be/src/exprs/expr.h:

Line 159:   virtual BooleanVal GetBooleanVal(ExprContext* context, const TupleRow*);
> Not sure I understand this comment. These need to be overridden by subclass
i wrote that comment before i reacquainted myself with the approach to specialization in expr/exprctx/fnctx
(all specialization is in expr).

this conflicts with the abstractions we had in mind (expr for static data, exprevaluator/fnctx
for execution). i think we should have another, hopefully shorter, discussion about what we
ultimately want these classes to look like. we don't want to make the changes are part of
this patch, but it's good to know where we're headed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefdc9aeeba033355cb9497e3a5d2363627dcf2f3
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message