Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3360: Codegen inserting into runtime filters ......................................................................
Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/CMakeLists.txt File be/src/exec/CMakeLists.txt: Line 40: filter-context-ir.cc > Missing this file? Done http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/filter-context.cc File be/src/exec/filter-context.cc: Line 225: Constant* expr_type_arg = codegen->ConstantToGVPtr( > This does look like it should work to me. I think the types should line up Yeah, I have no idea why it doesn't work. The types definitely line up or it wouldn't pass verification, and as far as I can tell the type is being passed correctly, its just somehow corrupting/overwriting the actual value that's getting hashed. I've been playing around with it, and the only thing I can figure out that makes it work is to change the cross-complied function to assign the ColumnType to a local variable rather than just handling a ref to it, but of course that somewhat defeats the point here. Is there someone who knows more about llvm that can help me with this? I can post the generated IR somewhere for someone to look at. I've already spent quite a lot of time trying to get this to work, and I'm not sure what else to do since I'm finding llvm to be very poorly documented. http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exprs/scalar-expr.cc File be/src/exprs/scalar-expr.cc: Line 368: // Set the pointer to NULL in case it evaluates to NULL. > It seems like the main way this differs from other places is returning a nu I don't know what you mean by this. If we're going to replace ScalarExprEval::GetValue() with ReplaceCallSites, then we need another function to replace it with. -- To view, visit http://gerrit.cloudera.org:8080/8029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79cf23ad92dadaab996a50a2ca07ef9ebe8639bb Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Matthew Jacobs <mjac...@apache.org> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes