Jiawei Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13680 )
Change subject: IMPALA-8665 Include extra info in error message when date cast fails ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/13680/2/be/src/exprs/cast-functions-ir.cc File be/src/exprs/cast-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/13680/2/be/src/exprs/cast-functions-ir.cc@310 PS2, Line 310: DateVal CastFunctions::CastToDateVal(FunctionContext* ctx, const StringVal& val) { > Please cover this change with tests. If you grep for "String to Date parse I have already tested all the tests you mentioned. They all passed. The way tests handle this is to find a string match in the output. So without changing the old tests, this still passes. Do you want me to change the tests cases? (i.e. add the additional information for string match) http://gerrit.cloudera.org:8080/#/c/13680/2/be/src/exprs/cast-functions-ir.cc@314 PS2, Line 314: (char *) > The preferred way of converting here is using reinterpret_cast as seen abov Done http://gerrit.cloudera.org:8080/#/c/13680/2/be/src/exprs/cast-functions-ir.cc@316 PS2, Line 316: invalidVal > Have you tried to simply pass the char* here? Is that a null terminated str It's not working because StringVal.ptr is not a null-terminated string. http://gerrit.cloudera.org:8080/#/c/13680/2/be/src/exprs/cast-functions-ir.cc@316 PS2, Line 316: c_str() > Do you need to convert the return value of Substitute() to char*? The reason why I used Substitute and .c_str() is that I am following the other SetError() examples. The following is one of the example that use SetError() function to pass a variable inside a string. To use SetError, char* is required. agg_fn_ctx_->SetError(Substitute("UDA Serialize() and Finalize() must return same pointer as input for $0 intermediate", dst_slot_desc.type().DebugString()).c_str()); -- To view, visit http://gerrit.cloudera.org:8080/13680 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If800b7696515cd61afee27220c55ff2440a86f04 Gerrit-Change-Number: 13680 Gerrit-PatchSet: 2 Gerrit-Owner: Jiawei Wang <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jiawei Wang <[email protected]> Gerrit-Comment-Date: Thu, 20 Jun 2019 18:14:44 +0000 Gerrit-HasComments: Yes
