Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16227 )

Change subject: IMPALA-9984: Implement codegen for TupleIsNullPredicate
......................................................................


Patch Set 3:

(2 comments)

I found these small things that could be corrected. Do you think I should 
upload a new patch or is it better to leave it alone now?

http://gerrit.cloudera.org:8080/#/c/16227/3/be/src/exprs/tuple-is-null-predicate-ir.cc
File be/src/exprs/tuple-is-null-predicate-ir.cc:

http://gerrit.cloudera.org:8080/#/c/16227/3/be/src/exprs/tuple-is-null-predicate-ir.cc@21
PS3, Line 21: IR_NO_INLINE
I realised this is not needed.


http://gerrit.cloudera.org:8080/#/c/16227/3/be/src/exprs/tuple-is-null-predicate.cc
File be/src/exprs/tuple-is-null-predicate.cc:

http://gerrit.cloudera.org:8080/#/c/16227/3/be/src/exprs/tuple-is-null-predicate.cc@141
PS3, Line 141: // Returns a 4-byte integer constant to be used in codegen.
             :   auto CodegenInt = [codegen](int val) -> llvm::Constant* {
             :     return codegen->GetIntConstant(4, val, 0);
             :   };
We could use codegen->GetI32Constant(uint64_t) instead.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I410aa7ec762ca16f455bd7da1dce763c1a7b156e
Gerrit-Change-Number: 16227
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Aug 2020 10:34:42 +0000
Gerrit-HasComments: Yes

Reply via email to