impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1430,IMPALA-4878,IMPALA-4879: codegen native UDAs
Date Fri, 03 Feb 2017 20:11:47 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-1430,IMPALA-4878,IMPALA-4879: codegen native UDAs
......................................................................


Patch Set 15:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5161/13/be/src/exprs/agg-fn-evaluator.cc
File be/src/exprs/agg-fn-evaluator.cc:

Line 215:   RETURN_IF_ERROR(Expr::Open(input_expr_ctxs_, state));
> this seems wrong for merge/finalize.  in those cases, this will be the inte
Done


PS13, Line 535: K();
> seems like a strange place given this isn't a scalar function being loaded.
Agreed, I mainly didn't want to move it in the first iteration so that the diff was easier
to read. Moved to LlvmCodeGen.


PS13, Line 538: <FunctionContext::TypeDesc>
> what does this condition mean? that it's defined? if it's not defined, what
Added a comment


PS13, Line 541: _fn_arg_types.push_back(
> what is this saying? i can see why arg_types for InlineConstants() should o
Removed this comment.


PS13, Line 547: etInterme
> as we discussed in person, it doesn't seem right to use these types for Fun
Made this change (also filed a JIRA to track it).


http://gerrit.cloudera.org:8080/#/c/5161/13/be/src/exprs/scalar-fn-call.h
File be/src/exprs/scalar-fn-call.h:

Line 53:   virtual std::string DebugString() const;
> I'm fine with moving it to a different class, I mainly wanted to leave as-i
Moved to LlvmCodeGen


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

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

Mime
View raw message