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 <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
