Surya Hebbar 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:

(2 comments)

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 {
> Seems not much to change here since this is for the Kudu status. I replaced
Done. I meant creating a single macro instead of two, while doing string 
conversions inline like..

#define SET_ERROR(ctx, status_string, prefix)
  do {
    (ctx)->SetError((prefix + (status_string).c_str());
  } while (false)


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, \
> I prefer the assertion to be clearer in the test so that when an error occu
Sure. I meant something like TestStringValue method present here in the long 
term.

To compare given StringVal to "String", as wrong outputs are highlighted with 
EXPECT and call line numbers can be traced in both cases.



--
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 12:22:16 +0000
Gerrit-HasComments: Yes

Reply via email to