Skip to content

Merge streamed result set parts in execute_with_retries#862

Merged
vgvoleg merged 7 commits into
mainfrom
execute-with-retries-merge-result-sets
Jul 20, 2026
Merged

Merge streamed result set parts in execute_with_retries#862
vgvoleg merged 7 commits into
mainfrom
execute-with-retries-merge-result-sets

Conversation

@vgvoleg

@vgvoleg vgvoleg commented Jul 17, 2026

Copy link
Copy Markdown
Member

execute_with_retries materializes the query stream, but the query service splits one logical result set into several stream parts that share a result_set_index. Those parts were returned as separate ResultSet objects, so a large SELECT came back as multiple result sets instead of one.

Row-format (VALUE) parts that share an index are now concatenated back into a single result set, so a large SELECT is returned as one result set instead of one per stream chunk. ARROW parts are left untouched: each already carries its own schema and record-batch data and is independently decodable, so they stay as separate result sets rather than splicing opaque bytes together.

The stream is aggregated in a single pass (sync and async) without an intermediate list.

Closes: #863

Stream parts that share a result_set_index are concatenated back into a
single ResultSet, so execute_with_retries returns one result set per
SELECT regardless of its size instead of one per stream part.
@vgvoleg
vgvoleg requested a review from Copilot July 17, 2026 14:04
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.25%. Comparing base (3bcd540) to head (bace702).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
+ Coverage   82.18%   82.25%   +0.07%     
==========================================
  Files          99       99              
  Lines       12683    12714      +31     
  Branches     1235     1241       +6     
==========================================
+ Hits        10423    10458      +35     
+ Misses       1804     1801       -3     
+ Partials      456      455       -1     
Flag Coverage Δ
integration 80.11% <87.87%> (+0.04%) ⬆️
unit 47.27% <93.93%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ydb/aio/query/pool.py 94.63% <100.00%> (ø)
ydb/convert.py 87.46% <100.00%> (+1.63%) ⬆️
ydb/query/pool.py 92.45% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes QuerySessionPool.execute_with_retries (sync + async) returning multiple ResultSet objects for a single logical SELECT when the Query service splits one result set across multiple streamed response parts sharing the same result_set_index. It introduces a utility to merge those parts back into one result set per index.

Changes:

  • Aggregate streamed ResultSet parts by result_set_index in execute_with_retries (sync and async).
  • Add aggregate_result_sets_by_index in ydb.convert and unit tests for its merging behavior (rows, schema, truncated, Arrow data).
  • Add integration tests validating large SELECT results are returned as a single merged result set; document the fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ydb/query/pool.py Uses result-set aggregation in sync execute_with_retries.
ydb/aio/query/pool.py Uses result-set aggregation in async execute_with_retries.
ydb/convert.py Adds aggregate_result_sets_by_index to merge streamed parts sharing an index.
ydb/query/pool_test.py Adds unit tests validating aggregation semantics (rows/schema/truncation/data).
tests/query/test_query_session_pool.py Adds integration test ensuring large result sets are merged for sync pool.
tests/aio/query/test_query_session_pool.py Adds integration test ensuring large result sets are merged for async pool.
CHANGELOG.md Adds user-facing changelog entry for the fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ydb/convert.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread ydb/convert.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread ydb/query/pool_test.py
Comment thread ydb/convert.py
@vgvoleg
vgvoleg merged commit b30d45d into main Jul 20, 2026
31 checks passed
@vgvoleg
vgvoleg deleted the execute-with-retries-merge-result-sets branch July 20, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Additional result sets from python sdk

3 participants