impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
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:

File be/src/exprs/

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

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).
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Id1708eaa96eb76fb9bec5eeabf209f81c88eec2f
Gerrit-PatchSet: 15
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message