impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "anujphadke (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Date Thu, 03 Aug 2017 17:35:18 GMT
anujphadke has posted comments on this change.

Change subject: IMPALA-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7574/2/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:

PS2, Line 126: TextConverter::CodegenWriteSlot
> Do you need this part? If the error gets logged, won't it include where it 
oops it should be Impala-2689. I will fix the commit message. 
This shows up in runtime profile when codegen is disabled. 

Ex: from the profile -
ExecOption: TEXT Codegen Disabled: TextConverter::CodegenWriteSlot: Char isn't supported for
CodegenWriteSlot

I think it would be useful to print this. 
Let me know what you think? I will fix this in the next patch.


Line 281:        "WriteSlot function failed verification");
> Maybe call it "FinalizeFunction() failed"? The error message seems differen
oops it should be Impala-2689. I will fix the commit message in the next patch.

FinalizeFunction returns NULL when verification fails. We have a similar log message's elsewhere
too -
https://github.com/cloudera/Impala/commit/2be7a1723f0340367e050f7cafd8a65fab55e37e#diff-1a1c603752b59b3305d47ba0805f1ca2R597
I ll also append the "check logs" to the message.

Let me know what do you think? I will change it accordingly in the next patch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <aphadke@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: anujphadke <aphadke@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message