Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/22588 )
Change subject: IMPALA-13812: Fail query for certain errors related to AI functions ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/22588/5/be/src/exprs/ai-functions-ir.cc File be/src/exprs/ai-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/22588/5/be/src/exprs/ai-functions-ir.cc@55 PS5, Line 55: AI > nit: AI Done http://gerrit.cloudera.org:8080/#/c/22588/5/be/src/exprs/ai-functions.cc File be/src/exprs/ai-functions.cc: http://gerrit.cloudera.org:8080/#/c/22588/5/be/src/exprs/ai-functions.cc@71 PS5, Line 71: } while (false) : : // Check the status and return an error if it fails. : #define RETURN_STRINGVAL_IF_ERROR(ctx, stmt) \ : do { > Possibly reuse the SET_ERROR macro here instead.. Seems not much to change here since this is for the Kudu status. I replaced the stringstream with string, which simplifies the code slightly while the performance should not be a big issue here. http://gerrit.cloudera.org:8080/#/c/22588/5/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/22588/5/be/src/exprs/expr-test.cc@11395 PS5, Line 11395: + AiFunctions::AI_GENERATE_TXT_COMMON_ERROR_PREFIX.size(); : size_t override_forbidden_error_len = : AiFunctions::AI_GENERATE_TXT_MSG_OVERRIDE_FORBIDDEN_ERROR.size() : + AiFunctions::AI_GENERATE_TXT_COMMON_ERROR_PREFIX.size(); : size_t n_override_forbidden_error_len = : AiFunctions::AI_GENERATE_TXT_N_OVERRIDE_FORBIDDEN_ERROR.size() : + AiFunctions::AI_GENERATE_TXT_COMMON_ERROR_PREFIX.size(); : // Test override/additional params : // invalid json results in error. : StringVal invalid_json_params("{\"temperature\": 0.49, \ > Maybe a common method to compare StringVal with a given string, as this pat I prefer the assertion to be clearer in the test so that when an error occurs, it directly points to the line where it fails rather than a common function. I wrapped the context close and creation into a helper function. -- To view, visit http://gerrit.cloudera.org:8080/22588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639e48e64d62f7990cf9a3c35a59a0ee3a2c64e0 Gerrit-Change-Number: 22588 Gerrit-PatchSet: 6 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 13 Mar 2025 00:26:24 +0000 Gerrit-HasComments: Yes
