Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9154 )

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9154/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9154/2//COMMIT_MSG@12
PS2, Line 12: failsand
nit: fails and


http://gerrit.cloudera.org:8080/#/c/9154/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/9154/2/be/src/codegen/llvm-codegen.cc@280
PS2, Line 280:  ss << "Could not parse module " << module_name << ": " << 
tmp_module.getError();
since the diagnostic handler is called, it would be worth adding the message 
here. Please see LinkModuleFromLocalFs() (Line:318 onwards) for an example of 
how to fetch the diagnostic message from the handler.


http://gerrit.cloudera.org:8080/#/c/9154/2/be/src/codegen/llvm-codegen.cc@1315
PS2, Line 1315: Status status = CreateFromFile(nullptr, &pool, nullptr, file, 
module_id, &codegen);
              :   if (!status.ok()) {
              :     codegen->Close();
              :     return status;
              :   }
i hit the same problem earlier when I tried loading a lib generated from an 
updated version of llvm into a lower version, glad this got fixed here.
Seems like this happens whenever llvm encounters a malformed file. Is there 
anyway we can add a test for this that doesn't involve adding a file? If not, 
would it make sense to add one, just for this test?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke <apha...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jan 2018 00:12:05 +0000
Gerrit-HasComments: Yes

Reply via email to