[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16317/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Wed, 12 Jun 2024 00:07:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 6: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21493/6/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/21493/6/be/src/exprs/expr-test.cc@11257 PS6, Line 11257: AiGenerateText It seems we don't have a testcase for AiFunctions::AiGenerateTextDefault, can we add one more test using AiFunctions::AiGenerateTextDefault? -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Wed, 12 Jun 2024 00:02:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has posted comments on this change. (
http://gerrit.cloudera.org:8080/21493 )
Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of
'Authorization' in the request header
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc
File be/src/exprs/ai-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc@146
PS4, Line 146: StringVal AiFunctions::AiGenerateText(FunctionContext* ctx,
const StringVal& endpoint,
: const StringVal& prompt, const StringVal& model,
: const StringVal& api_key_jceks_secret, const StringVal&
params) {
: std::string_view endpoint_sv(FLAGS_ai_endpoint);
: // endpoint validation
: if (endpoint.ptr != nullptr && endpoint.len != 0) {
: endpoint_sv =
std::string_view(reinterpret_cast(endpoint.ptr), endpoint.len);
: // Simple validation for endpoint. It should start with
https://
: if (!is_api_endpoint_valid(endpoint_sv)) {
: LOG(ERROR) << "AI Generate Text: \ninvalid protocol: " <<
endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_INVALID_PROTOCOL_ERROR.c_str());
: }
: }
: AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: default:
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
:
: StringVal AiFunctions::AiGenerateTextDefault(
: FunctionContext* ctx, const StringVal& prompt) {
: AI_PLATFORM platform =
GetAiPlatformFromEndpoint(FLAGS_ai_endpoint);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(ctx, FLAGS_ai_endpoint,
: prompt, NULL_STRINGVAL, NULL_STRINGVAL,
NULL_STRINGVAL, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(ctx,
: FLAGS_ai_endpoint, prompt, NULL_STRINGVAL,
NULL_STRINGVAL, NULL_STRINGVAL,
: false);
: default:
: DCHECK(false) << "Default endpoint " << FLAGS_ai_endpoint
<< "must be supported";
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< FLAGS_ai_endpoint;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
> I see. How about using a template for use_fast_path?
Done
--
To view, visit http://gerrit.cloudera.org:8080/21493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5
Gerrit-Change-Number: 21493
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Tue, 11 Jun 2024 23:43:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header Updated the POST request when communicating with Azure Open AI endpoint. The header now includes 'api-key: ' instead of 'Authorization: Bearer '. Also, removed 'model' as a required param for the Azure Open AI api call. This is mainly because the endpoint contains deployment which is basically already mapped to a model. Testing: - Updated existing unit test as per the Azure API reference - Manually tested builtin 'ai_generate_text' using an Azure Open AI deployment. Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 --- M be/src/exprs/ai-functions-ir.cc M be/src/exprs/ai-functions.h M be/src/exprs/ai-functions.inline.h M be/src/exprs/expr-test.cc 4 files changed, 170 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/21493/6 -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-13106: Support larger imported query profile sizes through compression
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21463 )
Change subject: IMPALA-13106: Support larger imported query profile sizes
through compression
..
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/queries/profileParseWorker.js
File www/scripts/queries/profileParseWorker.js:
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/queries/profileParseWorker.js@22
PS4, Line 22: try {
: var profile = JSON.parse(e.data).contents;
: var val = profile.profile_name;
: var query = {};
: query.id = val.substring(val.indexOf("=") + 1, val.length -
1);
: query.user = profile.child_profiles[0].info_strings
: .find(({key}) => key === "User").value;
: query.default_db = profile.child_profiles[0].info_strings
: .find(({key}) => key === "Default Db").value;
: query.type = profile.child_profiles[0].info_strings
: .find(({key}) => key === "Query Type").value;
: query.start_time = profile.child_profiles[0].info_strings
: .find(({key}) => key === "Start Time").value;
: query.end_time = profile.child_profiles[0].info_strings
: .find(({key}) => key === "End Time").value;
: query.bytes_read = profile.child_profiles[2].counters
: .find(({counter_name}) => counter_name ===
"TotalBytesRead").value;
: query.bytes_read = getReadableSize(query.bytes_read, 2);
: query.bytes_sent = profile.child_profiles[2].counters
: .find(({counter_name}) => counter_name ===
"TotalBytesSent").value;
: query.bytes_sent = getReadableSize(query.bytes_sent, 2);
: query.state = profile.child_profiles[0].info_strings
: .find(({key}) => key === "Query State").value;
: query.rows_fetched = profile.child_profiles[1].counters
: .find(({counter_name}) => counter_name ===
"NumRowsFetched").value;
: query.resource_pool = profile.child_profiles[0].info_strings
: .find(({key}) => key === "Request Pool").value;
: query.statement = profile.child_profiles[0].info_strings
: .find(({key}) => key === "Sql Statement").value;
: query.statement = query.statement.substring(0, 250) + "...";
: query.profile = pako.deflate(e.data, {level : 3});
: } catch (err) {
: query.error = err;
: }
What about wrapping these as a function, and test that function in
profileParseWorker.test.js?
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/tests/queries/profileParseWorker.test.js
File www/scripts/tests/queries/profileParseWorker.test.js:
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/tests/queries/profileParseWorker.test.js@28
PS4, Line 28: import("../../../pako.min.js").then((pako) => {
: pako = pako.default;
: expect(pako.inflate(pako.deflate(exampleJSONProfileText,
{level : 3}), {to : "string"}))
: .toBe(exampleJSONProfileText);
: });
> We cannot use/instantiate web workers in JEST framework or most other testi
Ack
--
To view, visit http://gerrit.cloudera.org:8080/21463
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c4f31beb9cac89051460bf764b6d50c3933bd03
Gerrit-Change-Number: 21463
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Surya Hebbar
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Comment-Date: Tue, 11 Jun 2024 23:35:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21484 )
Change subject: IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2984
PS8, Line 2984: } catch (ExecutionException e) {
> Sorry, I mean adding an error message for Preconditions.checkState(). Other
Ah, I misunderstood. Done.
--
To view, visit http://gerrit.cloudera.org:8080/21484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd
Gerrit-Change-Number: 21484
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Tue, 11 Jun 2024 22:43:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation
Hello Quanlong Huang, Daniel Becker, Riza Suminto, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21484 to look at the new patch set (#12). Change subject: IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation .. IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation isTrueWithNullSlots() can be expensive when it has to query the backend. Many of the expressions will look similar, especially in large auto-generated expressions. Adds a cache based on the nullified expression to avoid querying the backend for expressions with identical structure. With DEBUG logging enabled for the Analyzer, computes and logs stats about the null slots cache. Adds 'use_null_slots_cache' query option to disable caching. Documents the new option. Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M docs/impala.ditamap A docs/topics/impala_use_null_slots_cache.xml M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/planner/Planner.java 8 files changed, 129 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/21484/12 -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation .. Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2981 PS8, Line 2981: return globalState_.nullSlotsCache.get(nullTuplePred, > https://gerrit.cloudera.org/c/21483/6 implemented appropriate hashCodes for Ack http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2984 PS8, Line 2984: Preconditions.checkState(e.getCause() instanceof InternalException); > I'd rather not. This is preserving the original error message. Sorry, I mean adding an error message for Preconditions.checkState(). Otherwise, when it fails, clients just get an error of "IllegalStateException: null". -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 11 Jun 2024 22:05:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) Add DOAP file
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21449 ) Change subject: Add DOAP file .. Add DOAP file Adds the DOAP file so Impala can be listed in Apache projects with more information, e.g. https://projects.apache.org/projects.html?language#C++ Validated at https://www.w3.org/RDF/Validator/ Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 Reviewed-on: http://gerrit.cloudera.org:8080/21449 Reviewed-by: Michael Smith Tested-by: Quanlong Huang --- A doap_Impala.rdf 1 file changed, 156 insertions(+), 0 deletions(-) Approvals: Michael Smith: Looks good to me, approved Quanlong Huang: Verified -- To view, visit http://gerrit.cloudera.org:8080/21449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: merged Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 Gerrit-Change-Number: 21449 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR](asf-site) Add DOAP file
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21449 ) Change subject: Add DOAP file .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 Gerrit-Change-Number: 21449 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 11 Jun 2024 22:02:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/21493 )
Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of
'Authorization' in the request header
..
Patch Set 5: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc
File be/src/exprs/ai-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc@146
PS4, Line 146: StringVal AiFunctions::AiGenerateText(FunctionContext* ctx,
const StringVal& endpoint,
: const StringVal& prompt, const StringVal& model,
: const StringVal& api_key_jceks_secret, const StringVal&
params) {
: std::string_view endpoint_sv(FLAGS_ai_endpoint);
: // endpoint validation
: if (endpoint.ptr != nullptr && endpoint.len != 0) {
: endpoint_sv =
std::string_view(reinterpret_cast(endpoint.ptr), endpoint.len);
: // Simple validation for endpoint. It should start with
https://
: if (!is_api_endpoint_valid(endpoint_sv)) {
: LOG(ERROR) << "AI Generate Text: \ninvalid protocol: " <<
endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_INVALID_PROTOCOL_ERROR.c_str());
: }
: }
: AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: default:
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
:
: StringVal AiFunctions::AiGenerateTextDefault(
: FunctionContext* ctx, const StringVal& prompt) {
: AI_PLATFORM platform =
GetAiPlatformFromEndpoint(FLAGS_ai_endpoint);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(ctx, FLAGS_ai_endpoint,
: prompt, NULL_STRINGVAL, NULL_STRINGVAL,
NULL_STRINGVAL, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(ctx,
: FLAGS_ai_endpoint, prompt, NULL_STRINGVAL,
NULL_STRINGVAL, NULL_STRINGVAL,
: false);
: default:
: DCHECK(false) << "Default endpoint " << FLAGS_ai_endpoint
<< "must be supported";
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< FLAGS_ai_endpoint;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
> We can't. Note that there is template specialization and so you can't move
I see. How about using a template for use_fast_path?
template
StringVal AiFunctions::HandleAiGenerateText(FunctionContext* ctx,
const StringVal& endpoint, const StringVal& prompt, const StringVal& model,
const StringVal& api_key_jceks_secret, const StringVal& params) {
std::string_view endpoint_sv(FLAGS_ai_endpoint);
if (endpoint.ptr != nullptr && endpoint.len != 0) {
endpoint_sv = std::string_view(reinterpret_cast(endpoint.ptr),
endpoint.len);
if (!is_api_endpoint_valid(endpoint_sv)) {
LOG(ERROR) << "AI Generate Text: \ninvalid protocol: " << endpoint_sv;
return StringVal(AI_GENERATE_TXT_INVALID_PROTOCOL_ERROR.c_str());
}
}
AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
switch (platform) {
case AI_PLATFORM::OPEN_AI:
return AiGenerateTextInternal(
ctx, endpoint_sv, prompt, model, api_key_jceks_secret, params, false);
case AI_PLATFORM::AZURE_OPEN_AI:
return AiGenerateTextInternal(
ctx, endpoint_sv, prompt, model, api_key_jceks_secret, params, false);
default:
LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: " << endpoint_sv;
if (use_fast_path) {
DCHECK(false) << "Default endpoint " << FLAGS_ai_endpoint << "must be
supported";
}
return StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
}
}
StringVal AiFunctions::AiGenerateText(Funct
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16316/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 5 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 20:49:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has posted comments on this change. (
http://gerrit.cloudera.org:8080/21493 )
Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of
'Authorization' in the request header
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc
File be/src/exprs/ai-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc@146
PS4, Line 146: StringVal AiFunctions::AiGenerateText(FunctionContext* ctx,
const StringVal& endpoint,
: const StringVal& prompt, const StringVal& model,
: const StringVal& api_key_jceks_secret, const StringVal&
params) {
: std::string_view endpoint_sv(FLAGS_ai_endpoint);
: // endpoint validation
: if (endpoint.ptr != nullptr && endpoint.len != 0) {
: endpoint_sv =
std::string_view(reinterpret_cast(endpoint.ptr), endpoint.len);
: // Simple validation for endpoint. It should start with
https://
: if (!is_api_endpoint_valid(endpoint_sv)) {
: LOG(ERROR) << "AI Generate Text: \ninvalid protocol: " <<
endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_INVALID_PROTOCOL_ERROR.c_str());
: }
: }
: AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: default:
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
:
: StringVal AiFunctions::AiGenerateTextDefault(
: FunctionContext* ctx, const StringVal& prompt) {
: AI_PLATFORM platform =
GetAiPlatformFromEndpoint(FLAGS_ai_endpoint);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(ctx, FLAGS_ai_endpoint,
: prompt, NULL_STRINGVAL, NULL_STRINGVAL,
NULL_STRINGVAL, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(ctx,
: FLAGS_ai_endpoint, prompt, NULL_STRINGVAL,
NULL_STRINGVAL, NULL_STRINGVAL,
: false);
: default:
: DCHECK(false) << "Default endpoint " << FLAGS_ai_endpoint
<< "must be supported";
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< FLAGS_ai_endpoint;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
> Can we remove the duplicated code by using a new function, such as HandleAi
We can't. Note that there is template specialization and so you can't move this
into a common code block:
```
StringVal AiFunctions::AiGenerateText(.)
...
AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
switch(platform) {
case AI_PLATFORM::OPEN_AI:
return AiGenerateTextInternal(..);
case AI_PLATFORM::AZURE_OPEN_AI:
return AiGenerateTextInternal(.);
default:
...
}
```
```
StringVal AiFunctions::AiGenerateTextDefault()
...
AI_PLATFORM platform = GetAiPlatformFromEndpoint(FLAGS_ai_endpoint);
switch(platform) {
case AI_PLATFORM::OPEN_AI:
return AiGenerateTextInternal(.);
case AI_PLATFORM::AZURE_OPEN_AI:
return AiGenerateTextInternal(.);
default:
.
}
```
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h
File be/src/exprs/ai-functions.inline.h:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h@79
PS4, Line 79: Status status = getAuthorizationHeader(authHeader,
ai_api_key_);
: if (!status.ok()) return copyErrorMessage(ctx,
status.msg().msg());
> Is it okay if we pass the function pointer?
I think I had a missing parenthesis somewhere so the original macro seems to
work now.
--
To view, visit http://gerrit.cloudera.org:8080/21493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5
Gerrit-Change-Number: 21493
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header Updated the POST request when communicating with Azure Open AI endpoint. The header now includes 'api-key: ' instead of 'Authorization: Bearer '. Also, removed 'model' as a required param for the Azure Open AI api call. This is mainly because the endpoint contains deployment which is basically already mapped to a model. Testing: - Updated existing unit test as per the Azure API reference - Manually tested builtin 'ai_generate_text' using an Azure Open AI deployment. Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 --- M be/src/exprs/ai-functions-ir.cc M be/src/exprs/ai-functions.h M be/src/exprs/ai-functions.inline.h M be/src/exprs/expr-test.cc 4 files changed, 164 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/21493/5 -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 5 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 2: Please add tests to tests/query_test/test_observability.py and/or tests/webserver/test_web_pages.py. Also incldue example output in the commit message as it's not clear what human readable means. -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 11 Jun 2024 19:41:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13152: Avoid NaN cost at SortNode.java
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21504 ) Change subject: IMPALA-13152: Avoid NaN cost at SortNode.java .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16315/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib49c7ae397dadcb2cb69fde1850d442d33cdf177 Gerrit-Change-Number: 21504 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 11 Jun 2024 18:03:08 + Gerrit-HasComments: No
[Impala-ASF-CR] [tools] Add .gitignore for new files
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21503 ) Change subject: [tools] Add .gitignore for new files .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16314/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d289d465fff7c81091b28cd62b9436957f8bade Gerrit-Change-Number: 21503 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 17:55:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13152: Avoid NaN cost at SortNode.java
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21504 Change subject: IMPALA-13152: Avoid NaN cost at SortNode.java .. IMPALA-13152: Avoid NaN cost at SortNode.java TOP-N cost will turn into NaN if inputCardinality is equal to 0 due to Math.log(inputCardinality). This patch fix the issue by avoiding Math.log(0) and replace it with 0 instead. After this patch, Instantiating BaseProcessingCost with NaN or Infinite totalCost will throw IllegalArgumentException. In BaseProcessingCost.getDetails(), "total-cost" is renamed to "raw-cost" to avoid confusion with "cost-total" in ProcessingCost.getDetails(). Testing: - Add testcase that run TOP-N query over empty table. - Pass PlannerTest.testProcessingCost(). Change-Id: Ib49c7ae397dadcb2cb69fde1850d442d33cdf177 --- M fe/src/main/java/org/apache/impala/planner/BaseProcessingCost.java M fe/src/main/java/org/apache/impala/planner/ProcessingCost.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-processing-cost.test M testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-q43-verbose.test 5 files changed, 115 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/21504/1 -- To view, visit http://gerrit.cloudera.org:8080/21504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib49c7ae397dadcb2cb69fde1850d442d33cdf177 Gerrit-Change-Number: 21504 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto
[Impala-ASF-CR] [tools] Add .gitignore for new files
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21503 Change subject: [tools] Add .gitignore for new files .. [tools] Add .gitignore for new files Adds .gitignore for test.jceks - added with IMPALA-12920 - and hive-site-housekeeping-on (presumably added via a Hive update). Change-Id: I3d289d465fff7c81091b28cd62b9436957f8bade --- M fe/src/test/resources/.gitignore A testdata/jceks/.gitignore D testdata/jceks/.gitkeep 3 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/21503/1 -- To view, visit http://gerrit.cloudera.org:8080/21503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3d289d465fff7c81091b28cd62b9436957f8bade Gerrit-Change-Number: 21503 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/21493 )
Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of
'Authorization' in the request header
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h
File be/src/exprs/ai-functions.inline.h:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h@79
PS4, Line 79: Status status = getAuthorizationHeader(authHeader,
ai_api_key_);
: if (!status.ok()) return copyErrorMessage(ctx,
status.msg().msg());
> It looks like gcc doesn't like passing templatized function as an argument
Is it okay if we pass the function pointer?
#define RETURN_STRINGVAL_IF_ERROR(ctx, stmt, errorFunc)\
do { \
const ::impala::Status& _status = (stmt); \
if (UNLIKELY(!_status.ok())) \
return (errorFunc)(ctx, _status.msg().msg());\
} while (false)
RETURN_STRINGVAL_IF_ERROR(ctx, getAuthorizationHeader(authHeader,
ai_api_key_), copyErrorMessage);
--
To view, visit http://gerrit.cloudera.org:8080/21493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5
Gerrit-Change-Number: 21493
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Tue, 11 Jun 2024 16:53:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/21493 )
Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of
'Authorization' in the request header
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc
File be/src/exprs/ai-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions-ir.cc@146
PS4, Line 146: StringVal AiFunctions::AiGenerateText(FunctionContext* ctx,
const StringVal& endpoint,
: const StringVal& prompt, const StringVal& model,
: const StringVal& api_key_jceks_secret, const StringVal&
params) {
: std::string_view endpoint_sv(FLAGS_ai_endpoint);
: // endpoint validation
: if (endpoint.ptr != nullptr && endpoint.len != 0) {
: endpoint_sv =
std::string_view(reinterpret_cast(endpoint.ptr), endpoint.len);
: // Simple validation for endpoint. It should start with
https://
: if (!is_api_endpoint_valid(endpoint_sv)) {
: LOG(ERROR) << "AI Generate Text: \ninvalid protocol: " <<
endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_INVALID_PROTOCOL_ERROR.c_str());
: }
: }
: AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(
: ctx, endpoint_sv, prompt, model, api_key_jceks_secret,
params, false);
: default:
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< endpoint_sv;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
:
: StringVal AiFunctions::AiGenerateTextDefault(
: FunctionContext* ctx, const StringVal& prompt) {
: AI_PLATFORM platform =
GetAiPlatformFromEndpoint(FLAGS_ai_endpoint);
: switch(platform) {
: case AI_PLATFORM::OPEN_AI:
: return AiGenerateTextInternal(ctx, FLAGS_ai_endpoint,
: prompt, NULL_STRINGVAL, NULL_STRINGVAL,
NULL_STRINGVAL, false);
: case AI_PLATFORM::AZURE_OPEN_AI:
: return AiGenerateTextInternal(ctx,
: FLAGS_ai_endpoint, prompt, NULL_STRINGVAL,
NULL_STRINGVAL, NULL_STRINGVAL,
: false);
: default:
: DCHECK(false) << "Default endpoint " << FLAGS_ai_endpoint
<< "must be supported";
: LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: "
<< FLAGS_ai_endpoint;
: return
StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
: }
: }
Can we remove the duplicated code by using a new function, such as
HandleAiGenerateText()?
StringVal AiFunctions::HandleAiGenerateText(FunctionContext* ctx,
const StringVal& endpoint, const StringVal& prompt, const StringVal& model,
const StringVal& api_key_jceks_secret, const StringVal& params) {
std::string_view endpoint_sv(FLAGS_ai_endpoint);
if (endpoint.ptr != nullptr && endpoint.len != 0) {
endpoint_sv = std::string_view(reinterpret_cast(endpoint.ptr),
endpoint.len);
if (!is_api_endpoint_valid(endpoint_sv)) {
LOG(ERROR) << "AI Generate Text: \ninvalid protocol: " << endpoint_sv;
return StringVal(AI_GENERATE_TXT_INVALID_PROTOCOL_ERROR.c_str());
}
}
AI_PLATFORM platform = GetAiPlatformFromEndpoint(endpoint_sv);
switch (platform) {
case AI_PLATFORM::OPEN_AI:
return AiGenerateTextInternal(
ctx, endpoint_sv, prompt, model, api_key_jceks_secret, params, false);
case AI_PLATFORM::AZURE_OPEN_AI:
return AiGenerateTextInternal(
ctx, endpoint_sv, prompt, model, api_key_jceks_secret, params, false);
default:
LOG(ERROR) << "AI Generate Text: \nunsupported endpoint: " << endpoint_sv;
return StringVal(AI_GENERATE_TXT_UNSUPPORTED_ENDPOINT_ERROR.c_str());
}
}
StringVal AiFunctions::AiGenerateText(FunctionContext* ctx, const StringVal&
endpoint,
const StringVal& prompt, const StringVal& model,
const StringVal& api_key_jceks_secret, const StringVal& params) {
return HandleAiGenerateText(ctx, endpoint, prompt, model,
api_key_jceks_secret, params);
}
StringVal AiFunctions::AiGenerateTextDefault(
FunctionContext* ctx, const StringVal& prompt) {
return HandleAiGenerateText(
ctx, StringVal(), prompt, NULL_STRINGVAL,
[Impala-ASF-CR] IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation
Hello Quanlong Huang, Daniel Becker, Riza Suminto, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21484 to look at the new patch set (#11). Change subject: IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation .. IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation isTrueWithNullSlots() can be expensive when it has to query the backend. Many of the expressions will look similar, especially in large auto-generated expressions. Adds a cache based on the nullified expression to avoid querying the backend for expressions with identical structure. With DEBUG logging enabled for the Analyzer, computes and logs stats about the null slots cache. Adds 'use_null_slots_cache' query option to disable caching. Documents the new option. Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M docs/impala.ditamap A docs/topics/impala_use_null_slots_cache.xml M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/planner/Planner.java 8 files changed, 126 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/21484/11 -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation .. Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/21484/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21484/9//COMMIT_MSG@7 PS9, Line 7: IMPALA-12800: Add null slots cache > nit: Can we use a better title, e.g. "Add cache for isTrueWithNullSlots() e Done http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2981 PS8, Line 2981: return globalState_.nullSlotsCache.get(nullTuplePred, > I think we can improve the hash key (maybe in a follow-up JIRA). Currently, https://gerrit.cloudera.org/c/21483/6 implemented appropriate hashCodes for all Exprs. http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2982 PS8, Line 2982: .booleanValue() > nit: Unnecessary unboxing. Need 4 spaces indent. Done http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2984 PS8, Line 2984: Preconditions.checkState(e.getCause() instanceof InternalException); > nit: can we add an error message for this? I'd rather not. This is preserving the original error message. guava Cache#get wraps any Throwable thrown by the loader function in an ExecutionException. I'm unwrapping it to throw the exception we originally threw. -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 11 Jun 2024 16:29:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h File be/src/exprs/ai-functions.inline.h: http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h@79 PS4, Line 79: Status status = getAuthorizationHeader(authHeader, ai_api_key_); : if (!status.ok()) return copyErrorMessage(ctx, status.msg().msg()); > nit. can we use a macro to simplify error handling in this case? If not, th It looks like gcc doesn't like passing templatized function as an argument to macros. I get following errors: error: unterminated argument list invoking macro "RETURN_STRINGVAL_IF_ERROR" -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 16:20:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13138: Never smallify existing StringValue objects, only new ones during DeepCopy
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/21502 )
Change subject: IMPALA-13138: Never smallify existing StringValue objects, only
new ones during DeepCopy
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21502/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21502/1//COMMIT_MSG@18
PS1, Line 18: Testing:
Could you add some details about the repro steps?
http://gerrit.cloudera.org:8080/#/c/21502/1/be/src/runtime/string-value.h
File be/src/runtime/string-value.h:
http://gerrit.cloudera.org:8080/#/c/21502/1/be/src/runtime/string-value.h@72
PS1, Line 72: static StringValue MakeSmallStringFrom(const StringValue&
source) {
> This is only used in the buffered-tuple-stream-test? Are there other places
I haven't found any usages for this (except in test code). Does this mean that
we turn off Small String Optimisation totally for now?
http://gerrit.cloudera.org:8080/#/c/21502/1/tests/query_test/test_join_queries.py
File tests/query_test/test_join_queries.py:
http://gerrit.cloudera.org:8080/#/c/21502/1/tests/query_test/test_join_queries.py@214
PS1, Line 214: def test_spilling_hash_join(self, vector, unique_database):
Could you please add a comment about what this test does?
--
To view, visit http://gerrit.cloudera.org:8080/21502
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I739048b37a59a81c41c85d475fad00cb520a5f99
Gerrit-Change-Number: 21502
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Tue, 11 Jun 2024 16:01:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) Add DOAP file
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21449 ) Change subject: Add DOAP file .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 Gerrit-Change-Number: 21449 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 11 Jun 2024 15:37:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13138: Never smallify existing StringValue objects, only new ones during DeepCopy
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21502 )
Change subject: IMPALA-13138: Never smallify existing StringValue objects, only
new ones during DeepCopy
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21502/1/be/src/runtime/string-value.h
File be/src/runtime/string-value.h:
http://gerrit.cloudera.org:8080/#/c/21502/1/be/src/runtime/string-value.h@72
PS1, Line 72: static StringValue MakeSmallStringFrom(const StringValue&
source) {
This is only used in the buffered-tuple-stream-test? Are there other places it
could clean up some code?
--
To view, visit http://gerrit.cloudera.org:8080/21502
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I739048b37a59a81c41c85d475fad00cb520a5f99
Gerrit-Change-Number: 21502
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Tue, 11 Jun 2024 15:03:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13138: Never smallify existing StringValue objects, only new ones during DeepCopy
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21502 ) Change subject: IMPALA-13138: Never smallify existing StringValue objects, only new ones during DeepCopy .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16313/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I739048b37a59a81c41c85d475fad00cb520a5f99 Gerrit-Change-Number: 21502 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 11 Jun 2024 15:01:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 3: Code-Review+1 (4 comments) LGTM. Just a couple of nits. Note: It is important to set the right query option when impala is being used along with other applications (e.g. standalone java application) that may be following its own convention for conversion. http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@7 PS3, Line 7: allow nit: Allow http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@7 PS3, Line 7: writing nit: writing to http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@9 PS3, Line 9: commit nit: commit, http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift@946 PS2, Line 946: // written to Kudu as UNI > I agree that the current names are not too good, but "CONVERT_KUDU_UTC_TIME Thanks for the clarification. -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 11 Jun 2024 12:58:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13001: Support graceful and force shutdown for impala.sh.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/21297 ) Change subject: IMPALA-13001: Support graceful and force shutdown for impala.sh. .. Patch Set 4: Code-Review+1 LGTM! -- To view, visit http://gerrit.cloudera.org:8080/21297 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib7743234952ba6b12694ecc68a920d59fea0d4ba Gerrit-Change-Number: 21297 Gerrit-PatchSet: 4 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 11 Jun 2024 11:17:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13106: Support larger imported query profile sizes through compression
Surya Hebbar has posted comments on this change. (
http://gerrit.cloudera.org:8080/21463 )
Change subject: IMPALA-13106: Support larger imported query profile sizes
through compression
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/tests/queries/profileParseWorker.test.js
File www/scripts/tests/queries/profileParseWorker.test.js:
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/tests/queries/profileParseWorker.test.js@26
PS4, Line 26: ../../..
> nit: Maybe use $IMPALA_HOME env var instead?
In many .tmpl files relative paths had been used previously. So, I have used
relative paths in all scripts and tests.
I believe it won't be a problem, unless the "www" folder was relocated, it
would require a lot of changes to paths, not just here.
Is it advisable to post a patch, to fix all paths everywhere in www?
http://gerrit.cloudera.org:8080/#/c/21463/4/www/scripts/tests/queries/profileParseWorker.test.js@28
PS4, Line 28: import("../../../pako.min.js").then((pako) => {
: pako = pako.default;
: expect(pako.inflate(pako.deflate(exampleJSONProfileText,
{level : 3}), {to : "string"}))
: .toBe(exampleJSONProfileText);
: });
> Instead of just testing the compress-decompress, what if this test instanti
We cannot use/instantiate web workers in JEST framework or most other testing
frameworks.
Hence, I only tested the library method's we are using for compression.
--
To view, visit http://gerrit.cloudera.org:8080/21463
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c4f31beb9cac89051460bf764b6d50c3933bd03
Gerrit-Change-Number: 21463
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Surya Hebbar
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Comment-Date: Tue, 11 Jun 2024 09:59:01 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16312/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Tue, 11 Jun 2024 09:51:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21501 Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() In StringVal::CopyFrom(), we take the 'len' parameter as a size_t, which is usually a 64-bit unsigned integer. We pass it to the constructor of StringVal, which takes it as an int, which is usually a 32-bit signed integer. The constructor then allocates memory for the length using the int value, but afterwards in CopyFrom(), we copy the buffer with the size_t length. If size_t is indeed 64 bits and int is 32 bits, and the value is truncated, we may copy more bytes that what we have allocated for the destination. Note that in the constructor of StringVal it is checked whether the length is greater than 1GB, but if the value is truncated because of the type conversion, the check doesn't necessarily catch it as the truncated value may be small. This change fixes the problem by doing the length check with 64 bit integers in StringVal::CopyFrom(). Testing: - added unit tests for StringVal::CopyFrom() in udf-test.cc. Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 --- M be/src/udf/udf-test.cc M be/src/udf/udf.cc M be/src/udf/udf.h 3 files changed, 91 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21501/1 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21045 )
Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong
number
..
Patch Set 8:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/21045/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21045/8//COMMIT_MSG@9
PS8, Line 9: The description of events-skipped metric is wrong. Some cases in
Add partition
: event ,the metric will also be increased, besides for some other
cases like alter
: partition the event is skipped and the log is printed but the
events-skipped metric
: is not increased.
Please format this to be 72 characters width. Add space after comma and remove
space before comma. Add periods when neededd.
Please also explain the solution.
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1764
PS8, Line 1764: // both old table not removed and new table not add, that
means we skip the
: // rename event process if one is true and the other is
false, that means we
: // need to use the
function(removeTableIfNotAddedLater/addTableIfNotRemovedLater)
: // to do the real event process.
nit: might be more clear if reword this to
"Only bump the skipped metric if the old table is not removed and the new table
is not added. Not doing this in other cases since we need to either remove the
old table or add the new table, which is processing the event."
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1811
PS8, Line 1811: if (isOlderEvent(null)) {
Should we also bump the skipped metric in this case and L1819 for isSelfEvent()?
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1842
PS8, Line 1842: if (notSkipped) {
Shouldn't this be !notSkipped?
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@241
PS8, Line 241: . B
nit: remove period "." and use lowercase "b". This should be the same sentence.
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4879
PS8, Line 4879: return -1;
Could you add a comment about this? Why should we use -1 here?
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5018
PS8, Line 5018: ,
nit: add a space after comma ","
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@1995
PS8, Line 1995: // is IncompleteTable and addPartition event may also skip.
Do you mean sometimes we could see more events being skipped, e.g. 3 or 4?
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2033
PS8, Line 2033: assertTrue("Event fetch duration should be greater than
zero",
: response.getEvents_fetch_duration_mean() > 0);
: assertTrue("Event process duration should be greater than
zero",
: response.getEvents_process_duration_mean() > 0);
These two are unrelative to the skipped metric. I think we can remove them.
http://gerrit.cloudera.org:8080/#/c/21045/8/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2043
PS8, Line 2043: // alter event process will skip;
nit: remove semicolon ";" and reword to "ALTER events will be skipped."
--
To view, visit http://gerrit.cloudera.org:8080/21045
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a
Gerrit-Change-Number: 21045
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Anonymous
[Impala-ASF-CR] IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20221 ) Change subject: IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled .. IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled Before this patch, Impala checked whether the Impala service user had the WRITE access to the target HDFS table/partition(s) during the analysis of the INSERT and LOAD DATA statements in the legacy catalog mode. The access levels of the corresponding HDFS table and partitions were computed by the catalog server solely based on the HDFS permissions and ACLs when the table and partitions were instantiated. After this patch, we skip loading HDFS permissions and assume the Impala service user has the READ_WRITE permission on all the HDFS paths associated with the target table during query analysis when Ranger is enabled. The assumption could be removed after Impala's implementation of FsPermissionChecker could additionally take Ranger's policies of HDFS into consideration when performing the check. Testing: - Added end-to-end tests to verify Impala's behavior with respect to the INSERT and LOAD DATA statements when Ranger is enabled in the legacy catalog mode. Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Reviewed-on: http://gerrit.cloudera.org:8080/20221 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/rat_exclude_files.txt M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java A testdata/data/load_data_with_catalog_v1.txt M tests/authorization/test_ranger.py 4 files changed, 163 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/20221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Gerrit-Change-Number: 20221 Gerrit-PatchSet: 16 Gerrit-Owner: Halim Kim Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Halim Kim Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20221 ) Change subject: IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled .. Patch Set 15: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/20221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Gerrit-Change-Number: 20221 Gerrit-PatchSet: 15 Gerrit-Owner: Halim Kim Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Halim Kim Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 06:28:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13151: Use MonotonicNanos to track test time
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21497 ) Change subject: IMPALA-13151: Use MonotonicNanos to track test time .. IMPALA-13151: Use MonotonicNanos to track test time Uses MonotonicNanos to track test time rather than MonotonicStopWatch. IMPALA-2407 updated MonotonicStopWatch to use a low-precision implementation for performance, which on ARM in particular sometimes results in undercounting time by a few microseconds. That's enough to cause a failure in DataStreamTestSlowServiceQueue.TestPrioritizeEos. Also uses SleepForMs and NANOS_PER_SEC rather than Kudu versions to better match Impala code base. Reproduced on ARM and tested the new implementation for several dozen runs without failure. Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Reviewed-on: http://gerrit.cloudera.org:8080/21497 Reviewed-by: Riza Suminto Tested-by: Impala Public Jenkins --- M be/src/runtime/data-stream-test.cc 1 file changed, 11 insertions(+), 21 deletions(-) Approvals: Riza Suminto: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/21497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Gerrit-Change-Number: 21497 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13151: Use MonotonicNanos to track test time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21497 ) Change subject: IMPALA-13151: Use MonotonicNanos to track test time .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Gerrit-Change-Number: 21497 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 11 Jun 2024 03:21:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21483 ) Change subject: IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups .. IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups Adds a HashMap to ExprSubstitutionMap to speed lookups while retaining lists for correct ordering (ordering needs to match to SlotRef order). Ignores duplicate inserts, preserving the old behavior that only the first match would actually be usable; duplicates primarily show up as a result of combining duplicate distinct and aggregate expressions, or redundant nested aggregation (like the tests for IMPALA-10182). Implements localHash and hashCode for Expr and related classes. Avoids deep-cloning LHS Exprs in ExprSubstitutionMap as they're used for lookup and not expected to be mutated. Adds the many expressions test, which now runs in a handful of seconds. Change-Id: Ic538a82c69ee1dd76981fbacf95289c9d00ea9fe Reviewed-on: http://gerrit.cloudera.org:8080/21483 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java M fe/src/main/java/org/apache/impala/analysis/DateLiteral.java M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java M fe/src/main/java/org/apache/impala/analysis/FunctionName.java M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java M fe/src/main/java/org/apache/impala/analysis/Subquery.java M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java M fe/src/main/java/org/apache/impala/catalog/Function.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java A testdata/workloads/functional-planner/queries/PlannerTest/many-expression.test 27 files changed, 1,690 insertions(+), 86 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic538a82c69ee1dd76981fbacf95289c9d00ea9fe Gerrit-Change-Number: 21483 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21483 ) Change subject: IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic538a82c69ee1dd76981fbacf95289c9d00ea9fe Gerrit-Change-Number: 21483 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 11 Jun 2024 03:14:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12277: Fix NullPointerException for partitioned inserts when EP is turned off
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21437 )
Change subject: IMPALA-12277: Fix NullPointerException for partitioned inserts
when EP is turned off
..
Patch Set 12:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/21437/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/21437/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7205
PS12, Line 7205: public TUpdateCatalogResponse
updateCatalogImpl(TUpdateCatalogRequest update,
This method is super long now. It'd be nice to refactor it by extracting some
codes into methods.
http://gerrit.cloudera.org:8080/#/c/21437/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7262
PS12, Line 7262: FeCatalogUtils.loadAllPartitions((FeFsTable)table);
Instead of using the partition list in the cache, I think a cleaner approach is
always fetching the updated partition list from HMS (without loading the file
metadata). So we can also take care of new partitions added externally.
The reason is that we will always fetch the updated partition list later in
loadTableMetadata(). We can do it here to get rid of stale metadata cache.
http://gerrit.cloudera.org:8080/#/c/21437/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7497
PS12, Line 7497: return updateCatalogImpl(update, false); // we should
only retry once.
It's not clear to me whether all the stuffs in updateCatalogImpl() are
retryable, i.e. idempotent. E.g. do we fire duplicate HMS events?
http://gerrit.cloudera.org:8080/#/c/21437/12/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/21437/12/tests/custom_cluster/test_events_custom_configs.py@1279
PS12, Line 1279: self.client.execute("insert into {}.{}
partition(year=2024) values (0)"
Can we add more partitions to cover existing partitions? E.g. The table can
originally have several partitions. One is dropped externally. Another new one
(with a new name) is added externally. Then an INSERT in Impala inserts to all
these partitions.
--
To view, visit http://gerrit.cloudera.org:8080/21437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide8f1f6bf017e9a040b53bb5d5291ff2ea3e0d18
Gerrit-Change-Number: 21437
Gerrit-PatchSet: 12
Gerrit-Owner: Sai Hemanth Gantasala
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Sai Hemanth Gantasala
Gerrit-Comment-Date: Tue, 11 Jun 2024 03:04:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20221 ) Change subject: IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled .. Patch Set 14: Code-Review+2 Carry +1 from other reviewers. Thank Fang-Yu for making this merged! Thank Halim for the contribution! -- To view, visit http://gerrit.cloudera.org:8080/20221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Gerrit-Change-Number: 20221 Gerrit-PatchSet: 14 Gerrit-Owner: Halim Kim Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Halim Kim Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 01:21:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20221 ) Change subject: IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled .. Patch Set 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10705/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/20221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Gerrit-Change-Number: 20221 Gerrit-PatchSet: 15 Gerrit-Owner: Halim Kim Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Halim Kim Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 01:22:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20221 ) Change subject: IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled .. Patch Set 15: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/20221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Gerrit-Change-Number: 20221 Gerrit-PatchSet: 15 Gerrit-Owner: Halim Kim Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Halim Kim Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Tue, 11 Jun 2024 01:22:10 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Add DOAP file
Hello Laszlo Gaal, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21449 to look at the new patch set (#4). Change subject: Add DOAP file .. Add DOAP file Adds the DOAP file so Impala can be listed in Apache projects with more information, e.g. https://projects.apache.org/projects.html?language#C++ Validated at https://www.w3.org/RDF/Validator/ Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 --- A doap_Impala.rdf 1 file changed, 156 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/21449/4 -- To view, visit http://gerrit.cloudera.org:8080/21449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 Gerrit-Change-Number: 21449 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-13093: Fix failure in inserting into OBS tables
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21438 ) Change subject: IMPALA-13093: Fix failure in inserting into OBS tables .. Patch Set 3: It seems adding this to hdfs-site.xml can also fix the issue: fs.obs.file.visibility.enable true I'll check whether OBS returns the real block size. -- To view, visit http://gerrit.cloudera.org:8080/21438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2441da0fc521b4bbed10c8edceb937bde481 Gerrit-Change-Number: 21438 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 11 Jun 2024 00:38:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12800: Add null slots cache
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add null slots cache .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/21484/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21484/9//COMMIT_MSG@7 PS9, Line 7: IMPALA-12800: Add null slots cache nit: Can we use a better title, e.g. "Add cache for isTrueWithNullSlots() evaluation"? -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 11 Jun 2024 00:28:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12871: Added filtering capability for Calcite planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21498 ) Change subject: IMPALA-12871: Added filtering capability for Calcite planner .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16311/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If104bf1cd801d5ee92dd7e43d398a21a18be5d97 Gerrit-Change-Number: 21498 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 10 Jun 2024 23:34:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12871: Added filtering capability for Calcite planner
Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21498 Change subject: IMPALA-12871: Added filtering capability for Calcite planner .. IMPALA-12871: Added filtering capability for Calcite planner The Filter RelNode is now handled in the Calcite planner. The parsing and analysis is done by Calcite so there were no changes added to that portion. The ImpalaFilterRel class was created to handled the conversion of the Calcite LogicalFilter to create a filter condition within the Impala plan nodes. There is no explicit filter plan node in Impala. Instead, the filter condition attaches itself to an existing plan node. The filter condition gets passed into the children plan nodes through the ParentPlanRelContext. The ExprConjunctsConverter class is responsible for creating the filter Expr list that is used. The list contains separate AND conditions that are on the top level. Change-Id: If104bf1cd801d5ee92dd7e43d398a21a18be5d97 --- M java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ExprConjunctsConverter.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java M testdata/workloads/functional-query/queries/QueryTest/calcite.test 10 files changed, 267 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/21498/1 -- To view, visit http://gerrit.cloudera.org:8080/21498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If104bf1cd801d5ee92dd7e43d398a21a18be5d97 Gerrit-Change-Number: 21498 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Carlin
[Impala-ASF-CR] IMPALA-12871: Added filtering capability for Calcite planner
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/21498 )
Change subject: IMPALA-12871: Added filtering capability for Calcite planner
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java:
http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java@71
PS1, Line 71: private NodeWithExprs getChildPlanNode(ParentPlanRelContext
context) throws ImpalaException {
line too long (95 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/21498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If104bf1cd801d5ee92dd7e43d398a21a18be5d97
Gerrit-Change-Number: 21498
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Mon, 10 Jun 2024 23:09:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21492 )
Change subject: IMPALA-12370: allow converting timestamps to UTC when writing
Kudu
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@16
PS3, Line 16: To be able to read back Kudu tables written by Impala correctly
: convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to
: have the same value.
> Add validation in impala::ValidateQueryOptions.
Maybe a PlannerTest to see that to_utc_timestamp / utc_to_unix_micros is added
in query plan is also good addition?
http://gerrit.cloudera.org:8080/#/c/21492/3/fe/src/main/java/org/apache/impala/util/ExprUtil.java
File fe/src/main/java/org/apache/impala/util/ExprUtil.java:
http://gerrit.cloudera.org:8080/#/c/21492/3/fe/src/main/java/org/apache/impala/util/ExprUtil.java@126
PS3, Line 126: public static Expr toUtcTimestampExpr(Analyzer analyzer, Expr
timestampExpr,
: Boolean expectPreIfNonUnique) throws AnalysisException {
Add comment for this method. I think this wrap timestampExpr inside a
FunctionCallExpr?
--
To view, visit http://gerrit.cloudera.org:8080/21492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1
Gerrit-Change-Number: 21492
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Mon, 10 Jun 2024 22:32:33 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13151: Use MonotonicNanos to track test time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21497 ) Change subject: IMPALA-13151: Use MonotonicNanos to track test time .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16310/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Gerrit-Change-Number: 21497 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 22:29:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13151: Use MonotonicNanos to track test time
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21497 ) Change subject: IMPALA-13151: Use MonotonicNanos to track test time .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Gerrit-Change-Number: 21497 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 22:11:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21483 ) Change subject: IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic538a82c69ee1dd76981fbacf95289c9d00ea9fe Gerrit-Change-Number: 21483 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 22:13:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21483 ) Change subject: IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10704/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic538a82c69ee1dd76981fbacf95289c9d00ea9fe Gerrit-Change-Number: 21483 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 22:13:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13151: Use MonotonicNanos to track test time
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21497 ) Change subject: IMPALA-13151: Use MonotonicNanos to track test time .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10703/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Gerrit-Change-Number: 21497 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 22:12:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13151: Use MonotonicNanos to track test time
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21497 Change subject: IMPALA-13151: Use MonotonicNanos to track test time .. IMPALA-13151: Use MonotonicNanos to track test time Uses MonotonicNanos to track test time rather than MonotonicStopWatch. IMPALA-2407 updated MonotonicStopWatch to use a low-precision implementation for performance, which on ARM in particular sometimes results in undercounting time by a few microseconds. That's enough to cause a failure in DataStreamTestSlowServiceQueue.TestPrioritizeEos. Also uses SleepForMs and NANOS_PER_SEC rather than Kudu versions to better match Impala code base. Reproduced on ARM and tested the new implementation for several dozen runs without failure. Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 --- M be/src/runtime/data-stream-test.cc 1 file changed, 11 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/21497/1 -- To view, visit http://gerrit.cloudera.org:8080/21497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9beb63669c5bdd910e5f713ecd42551841e95400 Gerrit-Change-Number: 21497 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16309/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 10 Jun 2024 21:58:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@16 PS3, Line 16: To be able to read back Kudu tables written by Impala correctly : convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to : have the same value. Add validation in impala::ValidateQueryOptions. Maybe test that with CTAS query where both origin and target table is in Kudu and convert_kudu_utc_timestamps != write_kudu_utc_timestamps. http://gerrit.cloudera.org:8080/#/c/21492/3/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/3/common/thrift/ImpalaService.thrift@883 PS3, Line 883: conversiyon nit: conversion -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 10 Jun 2024 21:50:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/21455/5/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: http://gerrit.cloudera.org:8080/#/c/21455/5/be/src/exec/hash-table.cc@376 PS5, Line 376: _exprs_) / sam > If I understand correctly than capacity_ can be 1 or N*64. Isn't it possibl Change the formula to use double in patch set 7. The -8 in max_capacity calculation is to account for 1 extra BitMap overhead. Therefore, if mem_per_row = 4KB, max_capacity will be equal to 63.998046875. Thus, expect capacity_ = 63. -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 10 Jun 2024 21:35:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Hello Yida Wu, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21455 to look at the new patch set (#7). Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB ExprValuesCache uses BATCH_SIZE as a deciding factor to set its capacity. It bounds the capacity such that expr_values_array_ memory usage stays below 256KB. This patch tightens that limit to include all memory usage from ExprValuesCache::MemUsage() instead of expr_values_array_ only. Therefore, setting a very high BATCH_SIZE will not push the total memory usage of ExprValuesCache beyond 256KB. Testing: - Add test_join_queries.py::TestExprValueCache. - Pass core tests. Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 --- M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M bin/rat_exclude_files.txt A testdata/workloads/tpcds_partitioned/queries M tests/common/test_dimensions.py M tests/query_test/test_join_queries.py 6 files changed, 54 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/21455/7 -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy
[native-toolchain-CR] IMPALA-12541: Build GCC with --enable-linker-build-id
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21469 ) Change subject: IMPALA-12541: Build GCC with --enable-linker-build-id .. IMPALA-12541: Build GCC with --enable-linker-build-id This builds GCC with --enable-linker-build-id so that binaries have Build ID specified. Build ID is needed to produce OS packages with separate debuginfo. This is particularly important for libstdc++, because it is not built as part of the regular Impala build. Testing: - Verified that resulting binaries have .note.gnu.build-id Change-Id: Ieb2017ba1a348a9e9e549fa3268635afa94ae6d0 Reviewed-on: http://gerrit.cloudera.org:8080/21469 Reviewed-by: Michael Smith Reviewed-by: Laszlo Gaal Tested-by: Joe McDonnell --- M source/gcc/build.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Michael Smith: Looks good to me, but someone else must approve Laszlo Gaal: Looks good to me, approved Joe McDonnell: Verified -- To view, visit http://gerrit.cloudera.org:8080/21469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ieb2017ba1a348a9e9e549fa3268635afa94ae6d0 Gerrit-Change-Number: 21469 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith
[native-toolchain-CR] IMPALA-12541: Build GCC with --enable-linker-build-id
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21469 ) Change subject: IMPALA-12541: Build GCC with --enable-linker-build-id .. Patch Set 1: Verified+1 I haven't seen any issues with this, so I'm going to merge it. -- To view, visit http://gerrit.cloudera.org:8080/21469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb2017ba1a348a9e9e549fa3268635afa94ae6d0 Gerrit-Change-Number: 21469 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 10 Jun 2024 19:52:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12800: Add null slots cache
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add null slots cache .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift@947 PS8, Line 947: USE_NULL_SLOTS_CACHE = 179 > Cloned expressions should match the same key (unless they substantially mod Ok, not an issue then. Mark this as resolved since I don't want to hold this patch too long. -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 19:51:28 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-13121: Switch to ccache 3.7.12
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21473 ) Change subject: IMPALA-13121: Switch to ccache 3.7.12 .. Patch Set 2: Verified+1 I have built the toolchain with these docker images and everything went fine. It fixes the issue I was seeing, so I'm going to go ahead with this. -- To view, visit http://gerrit.cloudera.org:8080/21473 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90d751445daa0dc298b634c1049d637a14afac40 Gerrit-Change-Number: 21473 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 10 Jun 2024 19:21:32 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-13121: Switch to ccache 3.7.12
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21473 ) Change subject: IMPALA-13121: Switch to ccache 3.7.12 .. IMPALA-13121: Switch to ccache 3.7.12 The docker images currently build and use ccache 3.3.3. Recently, we ran into a case where debuginfo was being generated even though the cflags ended with -g0. The ccache release history has this note for 3.3.5: - Fixed a regression where the original order of debug options could be lost. This upgrades ccache to 3.7.12 to address this issue. Ccache 3.7.12 is the last ccache release that builds using autotools. Ccache 4 moves to build with CMake. Adding a CMake dependency would be complicated at this stage, because some of the older OSes don't provide a new enough CMake in the package repositories. Since we don't really need the new features of Ccache 4+, this sticks with 3.7.12 for now. This reenables the check_ccache_works() logic in assert-dependencies-present.py. Testing: - Built docker images and ran a toolchain build - The newer ccache resolves the unexpected debuginfo issue Change-Id: I90d751445daa0dc298b634c1049d637a14afac40 Reviewed-on: http://gerrit.cloudera.org:8080/21473 Reviewed-by: Michael Smith Reviewed-by: Laszlo Gaal Tested-by: Joe McDonnell --- M docker/all/assert-dependencies-present.py M docker/all/postinstall.sh 2 files changed, 5 insertions(+), 4 deletions(-) Approvals: Michael Smith: Looks good to me, but someone else must approve Laszlo Gaal: Looks good to me, approved Joe McDonnell: Verified -- To view, visit http://gerrit.cloudera.org:8080/21473 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I90d751445daa0dc298b634c1049d637a14afac40 Gerrit-Change-Number: 21473 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-12800: Add null slots cache
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add null slots cache .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift@947 PS8, Line 947: USE_NULL_SLOTS_CACHE = 179 > AFAIK, Impala FE does Expr cloning several times, and I don't know how long Cloned expressions should match the same key (unless they substantially modify the clone). Expr overrides equals. -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 19:14:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12800: Add null slots cache
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add null slots cache .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift@947 PS8, Line 947: USE_NULL_SLOTS_CACHE = 179 > If they all have the same weight, might as well use maxSize. AFAIK, Impala FE does Expr cloning several times, and I don't know how long their lifetime is and when are they garbage collected. I'm worry if there is a case where Analyzer just keep adding new Keys into this cache and keep it referenced unnecessarily. -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 19:10:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 3: Code-Review+1 (2 comments) This looks good to me. However I haven't dug into the Impala's internals when converting and comparing timestamps. I hope corresponding functionality has enough test coverage, so if there was an issue, it would be caught by pre-commit tests. http://gerrit.cloudera.org:8080/#/c/21492/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/2//COMMIT_MSG@16 PS2, Line 16: To be able to read back Kudu tables written by Impala correctly : convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to : have the same value. > The reason for adding a new flag (and not changing the name of the old one) Ah, I see. That makes sense to me -- thanks for the clarification. http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift@882 PS2, Line 882: When true, UNIXTIME_MICRO columns read from Kudu will be interpreted as UTC an > TIMESTAMP vs UNIXTIME_MICROS: Impala uses TIMESTAMP as SQL type (and does n Thanks for the update -- this is stated with more clarity now. -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 10 Jun 2024 18:49:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12800: Add null slots cache
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 ) Change subject: IMPALA-12800: Add null slots cache .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21484/8/common/thrift/ImpalaService.thrift@947 PS8, Line 947: USE_NULL_SLOTS_CACHE = 179 > What do you think of using custom Guava Weigher that weight each entry as 1 If they all have the same weight, might as well use maxSize. We could make this an option to configure maxSize. It's a short-lived cache that scales based on number of expressions, so I don't expect the overall cost to be that high. I lean towards keeping the configuration simpler, but open to changing it to a query option that sets the cache max size. -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 18:05:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 10 Jun 2024 17:08:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 10 Jun 2024 17:02:41 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12686: Build the toolchain with basic debug information (-g1)
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/20870 ) Change subject: IMPALA-12686: Build the toolchain with basic debug information (-g1) .. Patch Set 3: I'm going to have a new upload for this. It turns out debug info for LLVM is very large for release builds, so I'll be turning that off. I'll also be turning off debug info for Kudu's toolchain build (which also includes LLVM). -- To view, visit http://gerrit.cloudera.org:8080/20870 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee2e264b281f83ebc226d9bf7d4e5a99a52f1fc6 Gerrit-Change-Number: 20870 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 10 Jun 2024 16:30:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13146: Download NodeJS from native toolchain
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21494 ) Change subject: IMPALA-13146: Download NodeJS from native toolchain .. IMPALA-13146: Download NodeJS from native toolchain Some test runs have had issues downloading the NodeJS tarball from the nodejs servers. This changes the test to download from our native toolchain to make this more reliable. This means that future upgrades to NodeJS will need to upload new tarballs to the native toolchain. Testing: - Ran x86_64/ARM javascript tests Change-Id: I1def801469cb68633e89b4a0f3c07a771febe599 Reviewed-on: http://gerrit.cloudera.org:8080/21494 Tested-by: Impala Public Jenkins Reviewed-by: Surya Hebbar Reviewed-by: Wenzhe Zhou --- M tests/run-js-tests.sh 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Impala Public Jenkins: Verified Surya Hebbar: Looks good to me, but someone else must approve Wenzhe Zhou: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/21494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1def801469cb68633e89b4a0f3c07a771febe599 Gerrit-Change-Number: 21494 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/21493 )
Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of
'Authorization' in the request header
..
Patch Set 4: Code-Review+1
(1 comment)
Thanks Abhishek. It looks good to me.
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h
File be/src/exprs/ai-functions.inline.h:
http://gerrit.cloudera.org:8080/#/c/21493/4/be/src/exprs/ai-functions.inline.h@79
PS4, Line 79: Status status = getAuthorizationHeader(authHeader,
ai_api_key_);
: if (!status.ok()) return copyErrorMessage(ctx,
status.msg().msg());
nit. can we use a macro to simplify error handling in this case? If not, the
current approach is also good to me.
#define RETURN_STRINGVAL_IF_ERROR(ctx, stmt) \
do { \
const ::impala::Status& _status = (stmt);\
if (UNLIKELY(!_status.ok())) \
return copyErrorMessage(ctx, _status.msg().msg()); \
} while (false)
--
To view, visit http://gerrit.cloudera.org:8080/21493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5
Gerrit-Change-Number: 21493
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Mon, 10 Jun 2024 16:05:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16308/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Mon, 10 Jun 2024 15:30:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21455/5/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: http://gerrit.cloudera.org:8080/#/c/21455/5/be/src/exec/hash-table.cc@376 PS5, Line 376: * sample_size; If I understand correctly than capacity_ can be 1 or N*64. Isn't it possible that the first jump (1-64) is too high in some cases, e.g. if the capacity would be 16 with the original calculation (16KB expr_values_bytes_per_row_)? -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 10 Jun 2024 15:26:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Surya Hebbar has posted comments on this change. (
http://gerrit.cloudera.org:8080/21482 )
Change subject: IMPALA-13137: Add additional client fetch metrics columns to
the queries page
..
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21482/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21482/1//COMMIT_MSG@9
PS1, Line 9: ,
> nit: ,
Done
http://gerrit.cloudera.org:8080/#/c/21482/1//COMMIT_MSG@10
PS1, Line 10: the
> nit: the
Done
http://gerrit.cloudera.org:8080/#/c/21482/1/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:
http://gerrit.cloudera.org:8080/#/c/21482/1/be/src/service/impala-http-handler.cc@460
PS1, Line 460: d
> nit: move ',' to previous line
Done
http://gerrit.cloudera.org:8080/#/c/21482/1/be/src/service/impala-http-handler.cc@462
PS1, Line 462: document->AddMember("tips_client_fetch_wait_time", "The time
taken for the client to"
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/21482/1/be/src/service/impala-http-handler.cc@551
PS1, Line 551: first_row_fetched_ns = re
> nit: it's already inited as 0, not necessary to set
Done
http://gerrit.cloudera.org:8080/#/c/21482/1/be/src/service/impala-http-handler.cc@567
PS1, Line 567: ec
> nit: move '' to previous line
Done
--
To view, visit http://gerrit.cloudera.org:8080/21482
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5
Gerrit-Change-Number: 21482
Gerrit-PatchSet: 2
Gerrit-Owner: Surya Hebbar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Surya Hebbar
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Comment-Date: Mon, 10 Jun 2024 15:06:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Surya Hebbar has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. IMPALA-13137: Add additional client fetch metrics columns to the queries page For helping users to better understand query execution times, the following columns have been added to the queries page. - First row fetched time - Client fetch wait time The duration needed to fetch the first row is already present in the query state record. This timestamp has to be found by matching the label "First row fetched" within the query state record's 'event_sequence'. The time taken for the client to fetch all rows is monitored by the "ClientFetchWaitTimer" in client request state. To incorporate this into the query state record, the following new attribute has been added. - client_fetch_wait_time_ns In order to constantly retrieve and update this value, a new method has been added to expose the private counter holding "ClientFetchWaitTimer". The duration for both these columns is printed in human readable format, after converting from nanoseconds. Tips for the two additional columns have also been added to the queries page. Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 --- M be/src/service/client-request-state.h M be/src/service/impala-http-handler.cc M be/src/service/query-state-record.cc M be/src/service/query-state-record.h M www/queries.tmpl 5 files changed, 47 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/21482/2 -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16307/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 10 Jun 2024 14:59:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21493/3/be/src/exprs/ai-functions.inline.h File be/src/exprs/ai-functions.inline.h: http://gerrit.cloudera.org:8080/#/c/21493/3/be/src/exprs/ai-functions.inline.h@75 PS3, Line 75: getAuthorizationHeader > If it is an unsupported platform, would it be better to throw an error and It's not possible to get 'UnSupported Platform' in getAuthorizationHeader() because we check for platform in the caller function and return error for unsupported platform. Added a Status check and error handling in case this assumption ever changes. http://gerrit.cloudera.org:8080/#/c/21493/3/be/src/exprs/ai-functions.inline.h@75 PS3, Line 75: getAuthorizationHeader() + api_key > Also would it be better to combine the api_key as a parameter to getAuthori Done -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 10 Jun 2024 14:37:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Abhishek Rawat has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header Updated the POST request when communicating with Azure Open AI endpoint. The header now includes 'api-key: ' instead of 'Authorization: Bearer '. Also, removed 'model' as a required param for the Azure Open AI api call. This is mainly because the endpoint contains deployment which is basically already mapped to a model. Testing: - Updated existing unit test as per the Azure API reference - Manually tested builtin 'ai_generate_text' using an Azure Open AI deployment. Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 --- M be/src/exprs/ai-functions-ir.cc M be/src/exprs/ai-functions.h M be/src/exprs/ai-functions.inline.h M be/src/exprs/expr-test.cc 4 files changed, 154 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/21493/4 -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/21492/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/2//COMMIT_MSG@16 PS2, Line 16: To be able to read back Kudu tables written by Impala correctly : convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to : have the same value. > +1 The reason for adding a new flag (and not changing the name of the old one) is backward compatibility. See discussion at IMPALA-12322 http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift@882 PS2, Line 882: When true, UNIXTIME_MICRO columns read from Kudu will be interpreted as UTC an > To be complete: TIMESTAMP vs UNIXTIME_MICROS: Impala uses TIMESTAMP as SQL type (and does not support DATETIME). Impala TIMESTAMPs have no timezone associated with individual values (or individual columns), only a "global" (per query) timezone that affects some conversions. Modified the comment, I hope that is is more precise now. http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift@944 PS2, Line 944: > Ditto: please consider updating this similar to the comment for CONVERT_KUD Modified the comment. http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift@946 PS2, Line 946: // written to Kudu as UNI > Make both the query names similar. For read, it starts with CONVERT_ and fo I agree that the current names are not too good, but "CONVERT_KUDU_UTC_TIMESTAMPS" is already released, so changing the name or the semantics could break existing workloads. http://gerrit.cloudera.org:8080/#/c/21492/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/21492/2/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@a3 PS2, Line 3: > what's 'kutu'? Done http://gerrit.cloudera.org:8080/#/c/21492/2/tests/common/test_result_verifier.py File tests/common/test_result_verifier.py: http://gerrit.cloudera.org:8080/#/c/21492/2/tests/common/test_result_verifier.py@427 PS2, Line 427: if file_format == 'avro' and 'TIMESTAMP' in expected_types: > I don't know much about the context here, but Kudu still doesn't support TI This is about verifying the results in our .test files. TIMESTAMP refers to the Impala data type returned by the query and file_format is about our test vector (we try to run the same queries with different file/table formats). e.g. this would also affect select cast(string_col as timestamp) from kudu_tbl; which doesn't even use UNIXTIME_MICRO. So this looks like an old hack to me to skip some tests Kudu/Avro in .test files while running the others. It was a total surprise when I realized that the results of the tests I wrote is actually not checked. This could still make sense for tests that use nanosecond timestamps as those are not supported in Kudu, but there are better tools to skip those tests. -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 10 Jun 2024 14:12:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21483 ) Change subject: IMPALA-12800: Use HashMap for ExprSubstitutionMap lookups .. Patch Set 4: Code-Review+2 Thanks Michael. -- To view, visit http://gerrit.cloudera.org:8080/21483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic538a82c69ee1dd76981fbacf95289c9d00ea9fe Gerrit-Change-Number: 21483 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 10 Jun 2024 14:09:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16306/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 10 Jun 2024 14:09:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Hello Alexey Serbin, Ashwani Raina, Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21492 to look at the new patch set (#3). Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. IMPALA-12370: allow converting timestamps to UTC when writing Kudu Before this commit only read support was implemented (convert_kudu_utc_timestamps=true). This change adds write support: if write_kudu_utc_timestamps=true, then timestamps are converted from local time to UTC during INSERT/UPSERT to Kudu. In case of ambigious conversions (DST changes) the earlier possible UTC timestamp is written. To be able to read back Kudu tables written by Impala correctly convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to have the same value. The conversion is implemented by adding to_utc_timestamp() to inserted timestamp expressions during planning. This allows doing the same conversion during the pre-insert sorting and partitioning. Read support is implemented differently - in that case the plan is not changed and the scanner does the conversion. Other changes: - Before this change, verification of tests with TIMESTAMP results were skipped when the file format is Kudu. This shouldn't be necessary so the skipping was removed. Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/util/ExprUtil.java M testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test M tests/common/test_result_verifier.py M tests/query_test/test_kudu.py 11 files changed, 101 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21492/3 -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] WIP: IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: WIP: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/21492/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/2//COMMIT_MSG@16 PS2, Line 16: To be able to read back Kudu tables written by Impala correctly : convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to : have the same value. > This leads to an obvious question: why to introduce an extra option --write +1 http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/2/common/thrift/ImpalaService.thrift@946 PS2, Line 946: WRITE_KUDU_UTC_TIMESTAMPS Make both the query names similar. For read, it starts with CONVERT_ and for write it is simply WRITE_ Would be more uniformity in names, if it looks something like of these options: READ_CONVERT_KUDU_UTC_TIMESTAMPS / WRITE_CONVERT_KUDU_UTC_TIMESTAMPS OR READ_KUDU_UTC_TIMESTAMPS / WRITE_KUDU_UTC_TIMESTAMPS or anything you seem fit where naming convention is uniform. -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 10 Jun 2024 08:58:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21478 )
Change subject: IMPALA-13120: Load failed table without need for manual
invalidate
..
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/21478/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21478/1//COMMIT_MSG@24
PS1, Line 24:
Could you add a summary on the solution?
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@169
PS1, Line 169: loadTables
nit: Is this actually "unloadedTables" ? Could you add a comment on the
relationship between this and 'missingTbls' ?
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@349
PS1, Line 349: missingTbls.addAll(getMissingTables(catalog, viewTbls,
loadTables));
nit: add brackets {}
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2470
PS1, Line 2470: !(tbl instanceof IncompleteTable)
Could you add a comment on why do we need this? Is there an IncompleteTable
that isLoaded() returns true?
EDIT: I realized IncompleteTable.isLoaded() returns (cause_ != null). However,
after this change, all IncompleteTable will be reloaded again regardless what
the cause_ is. Can we just do so for recoverable errors? Errors like
unsupported column types are unrecoverable. E.g. if I create a table with UNION
column types in Hive and use it in Impala:
hive> CREATE TABLE employee (
id INT,
name STRING,
salary DOUBLE,
additional_info UNIONTYPE);
impala> invalidate metadata employee;
impala> describe employee;
ERROR: AnalysisException: Could not load table default.employee from catalog
CAUSED BY: TableLoadingException: Could not load table default.employee from
catalog
CAUSED BY: TException:
TGetPartialCatalogObjectResponse(status:TStatus(status_code:GENERAL,
error_msgs:[TableLoadingException: Unsupported type
'uniontype' in column 'additional_info' of table
'employee']), lookup_status:OK)
It'd be better to keep the original behavior for such unrecoverable errors.
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java@32
PS1, Line 32: }
nit: format
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21478/1/fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java@91
PS1, Line 91: isLoadFailed
nit: can we emphasize the error is recoverable, e.g.
isLoadFailedByRecoverableError()?
I think HMS restart is just one cause of such failures. Restarts of other
services like NameNode, Kudu Master, HBase Master could also lead to
recoverable errors.
http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py
File tests/custom_cluster/test_catalog_hms_failures.py:
http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py@88
PS1, Line 88: @pytest.mark.execute_serially
Let's also test on local-catalog mode, i.e. adding parameters like
@CustomClusterTestSuite.with_args(
impalad_args='--use_local_catalog',
catalogd_args='--catalog_topic_mode=minimal')
http://gerrit.cloudera.org:8080/#/c/21478/1/tests/custom_cluster/test_catalog_hms_failures.py@91
PS1, Line 91: self.run_stmt_in_hive("create table {0} (i int)".format(table))
: EventProcessorUtils.wait_for_event_processing(self)
We can create the table using Impala. The table is unloaded after the creation.
--
To view, visit http://gerrit.cloudera.org:8080/21478
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15
Gerrit-Change-Number: 21478
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sai Hemanth Gantasala
Gerrit-Comment-Date: Mon, 10 Jun 2024 02:38:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sun, 09 Jun 2024 22:03:37 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Add DOAP file
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21449 ) Change subject: Add DOAP file .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21449 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ib1a47c68345769281449dd377a6f82a7257cee07 Gerrit-Change-Number: 21449 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sun, 09 Jun 2024 21:55:38 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-13072: Increase verbosity to print compilation commands
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21470 ) Change subject: IMPALA-13072: Increase verbosity to print compilation commands .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a27bfe401c5def341b2e52f7d5e77d7f0e3a19e Gerrit-Change-Number: 21470 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Sun, 09 Jun 2024 21:47:19 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12541: Build GCC with --enable-linker-build-id
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21469 ) Change subject: IMPALA-12541: Build GCC with --enable-linker-build-id .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb2017ba1a348a9e9e549fa3268635afa94ae6d0 Gerrit-Change-Number: 21469 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Sun, 09 Jun 2024 21:32:18 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-13121: Switch to ccache 3.7.12
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21473 ) Change subject: IMPALA-13121: Switch to ccache 3.7.12 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21473 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90d751445daa0dc298b634c1049d637a14afac40 Gerrit-Change-Number: 21473 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Sun, 09 Jun 2024 21:31:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: > Patch Set 16: Verified-1 > > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10701/ The failed test is due to IMPALA-12266. -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sun, 09 Jun 2024 17:01:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10702/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sun, 09 Jun 2024 17:01:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13146: Download NodeJS from native toolchain
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21494 ) Change subject: IMPALA-13146: Download NodeJS from native toolchain .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1def801469cb68633e89b4a0f3c07a771febe599 Gerrit-Change-Number: 21494 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Sun, 09 Jun 2024 16:21:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13146: Download NodeJS from native toolchain
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/21494 ) Change subject: IMPALA-13146: Download NodeJS from native toolchain .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21494 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1def801469cb68633e89b4a0f3c07a771febe599 Gerrit-Change-Number: 21494 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Surya Hebbar Gerrit-Comment-Date: Sun, 09 Jun 2024 09:40:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10701/ -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sun, 09 Jun 2024 07:35:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21493 ) Change subject: IMPALA-13131: Azure OpenAI API expects 'api-key' instead of 'Authorization' in the request header .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21493/3/be/src/exprs/ai-functions.inline.h File be/src/exprs/ai-functions.inline.h: http://gerrit.cloudera.org:8080/#/c/21493/3/be/src/exprs/ai-functions.inline.h@75 PS3, Line 75: getAuthorizationHeader If it is an unsupported platform, would it be better to throw an error and return earlier here instead of putting "Unsupported Platform" to the header? How about returning an empty string or null if not supported in getAuthorizationHeader(), so that the caller would take less cost to validate it, or maybe using a Status and a string pointer as the return from getAuthorizationHeader()? http://gerrit.cloudera.org:8080/#/c/21493/3/be/src/exprs/ai-functions.inline.h@75 PS3, Line 75: getAuthorizationHeader() + api_key Also would it be better to combine the api_key as a parameter to getAuthorizationHeader()? It seems more readable. -- To view, visit http://gerrit.cloudera.org:8080/21493 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9cc07940ce355d511bcf0ee615ff31042d13eb5 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Rawat Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Sun, 09 Jun 2024 02:55:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16305/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sun, 09 Jun 2024 02:57:32 + Gerrit-HasComments: No
