impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) Inject precision and scale constants for all decimal builtins
Date Thu, 17 Mar 2016 00:09:01 GMT
Tim Armstrong has posted comments on this change.

Change subject: Inject precision and scale constants for all decimal builtins
......................................................................


Patch Set 6:

(4 comments)

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

Line 637:     // Increment iter now so we don't mess it up modifying the instrunction below
> Can you fix 'instrunction' while you're here?
Done


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

Line 276:   // InlineConstants() must be run on the function to replace recognized constants.
The
> InlineConstants() doesn't have to be run, the interpreted path does not run
Done


Line 297:   int InlineConstants(LlvmCodeGen* codegen, llvm::Function* fn);
> Maybe keep this signature protected, since it's meant to be used by Expr su
Done


http://gerrit.cloudera.org:8080/#/c/2535/6/be/src/exprs/scalar-fn-call.cc
File be/src/exprs/scalar-fn-call.cc:

Line 486:     InlineConstants(return_type, arg_types, codegen, *udf);
> Use the original InlineConstants signature?
Ah, forgot to fix this up. Done.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b7d1485f357ba88517ca15c5c7428c0ffe25dfd
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message