Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21168 )
Change subject: IMPALA-12920: Support ai_generate_text built-in function for OpenAI's chat completion API ...................................................................... Patch Set 16: (3 comments) http://gerrit.cloudera.org:8080/#/c/21168/16/be/src/exprs/ai-functions.h File be/src/exprs/ai-functions.h: http://gerrit.cloudera.org:8080/#/c/21168/16/be/src/exprs/ai-functions.h@18 PS16, Line 18: #ifndef IMPALA_EXPRS_AI_FUNCTIONS_H : #define IMPALA_EXPRS_AI_FUNCTIONS_H > nit. #pragma once seems simpler yes please http://gerrit.cloudera.org:8080/#/c/21168/14/be/src/exprs/ai-functions.inline.h File be/src/exprs/ai-functions.inline.h: http://gerrit.cloudera.org:8080/#/c/21168/14/be/src/exprs/ai-functions.inline.h@43 PS14, Line 43: : namespace impala { : : template <bool fastpath> : StringVal AiFunctions::AiGenerateTextInternal(FunctionContext* ctx, : const StringVal& endpoint, const StringVal& prompt, const StringVal& model, : const StringVal& api_key_jceks_secret, const StringVal& params, const bool dry_run) { : std::string_view endpoint_sv(FLAGS_ai_endpoint); : // endpoint validation : if (!fastpath && endpoint.ptr != nullptr && endpoint.len != 0) { : endpoint_sv = std::string_view(reinterpret_cast<char*>(endpoint.p > missed static oh right, I missed that these were being initialized in a header file. http://gerrit.cloudera.org:8080/#/c/21168/16/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/21168/16/be/src/exprs/expr-test.cc@11341 PS16, Line 11341: string("A null-terminated string is a character string in a programming " : "language like C and C++ that ends with a null character (\'\\0\') . This " : "character represents the end of the string and is used to determine the " : "conclusion of the text. Essentially, it is a sequence of characters " : "followed by a null byte.") > nit: use content instead There are some slight differences in escaping. Could transform the string to remove them for this comparison. -- To view, visit http://gerrit.cloudera.org:8080/21168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4446957f6030bab1f985fdd69185c3da07d7c4b Gerrit-Change-Number: 21168 Gerrit-PatchSet: 16 Gerrit-Owner: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Wed, 10 Apr 2024 17:41:30 +0000 Gerrit-HasComments: Yes
