-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[Feature](sink) support parallel result sink #36053
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
run buildall |
PR approved by at least one committer and no changes requested. |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
PR approved by at least one committer and no changes requested. |
## Proposed changes support parallel result sink
This PR changed result sink logic: #36053 Need to change the show column stats get row batch logic to avoid NPE.
## Proposed changes optimization for parallel result sink #36053
## Proposed changes optimization for parallel result sink apache#36053
This PR changed result sink logic: #36053 Need to change the show column stats get row batch logic to avoid NPE.
## Proposed changes optimization for parallel result sink #36053
… and dry_run_query=true. (#53653) Related PR: #36053 && #53209 Problem Summary: Fixed the issue that the result of `dry_run_query=true` is wrong in multi-be scenarios (the number of rows is less) I think this may be related to the multi result sink introduced in pr #36053 && #53209. Reason: ```cpp vmysql_result_writer.cpp GetResultBatchCtx::on_close() { ... statistics->set_returned_rows(returned_rows); // Set only once per result_writer. ... } ``` ``` java if (connectContext != null && connectContext.getSessionVariable().dryRunQuery) { if (resultBatch.isEos()) { // This will only be counted once. numReceivedRows += resultBatch.getQueryStatistics().getReturnedRows(); } } else if (resultBatch.getBatch() != null) { numReceivedRows += resultBatch.getBatch().getRowsSize(); } ``` If there are multiple be, there will be multiple result sinks, and each result sink will update its own row count at the end. Since `resultBatch.isEos()` is only triggered once, the row count information is less.
… and dry_run_query=true. (apache#53653) Related PR: apache#36053 && apache#53209 Problem Summary: Fixed the issue that the result of `dry_run_query=true` is wrong in multi-be scenarios (the number of rows is less) I think this may be related to the multi result sink introduced in pr apache#36053 && apache#53209. Reason: ```cpp vmysql_result_writer.cpp GetResultBatchCtx::on_close() { ... statistics->set_returned_rows(returned_rows); // Set only once per result_writer. ... } ``` ``` java if (connectContext != null && connectContext.getSessionVariable().dryRunQuery) { if (resultBatch.isEos()) { // This will only be counted once. numReceivedRows += resultBatch.getQueryStatistics().getReturnedRows(); } } else if (resultBatch.getBatch() != null) { numReceivedRows += resultBatch.getBatch().getRowsSize(); } ``` If there are multiple be, there will be multiple result sinks, and each result sink will update its own row count at the end. Since `resultBatch.isEos()` is only triggered once, the row count information is less.
Proposed changes
support parallel result sink