Skip to content

Allow BatchSortedMerge with no segmentby or sorted on segmentbys pinned to Consts#9702

Open
natalya-aksman wants to merge 3 commits intotimescale:mainfrom
natalya-aksman:allow_batch_sorted_merge_with_no_segmentby
Open

Allow BatchSortedMerge with no segmentby or sorted on segmentbys pinned to Consts#9702
natalya-aksman wants to merge 3 commits intotimescale:mainfrom
natalya-aksman:allow_batch_sorted_merge_with_no_segmentby

Conversation

@natalya-aksman
Copy link
Copy Markdown
Member

@natalya-aksman natalya-aksman commented Apr 30, 2026

Addresses the easiest scenario for https://github.com/timescale/eng-database/issues/811: for unordered chunks, allow BatchSortedMerge for columnstores with no segmentby.
Also a trivial case when we sort on all (segmentby + orderby) when all segmentby columns are pinned to Consts, meaning we are sorting one segment only.

Some existing tests can use BatchSortedMerge now because of the above changes, those tests were recapped.

@natalya-aksman natalya-aksman requested a review from a team April 30, 2026 18:34
@github-actions github-actions Bot requested review from antekresic and dbeck April 30, 2026 18:35
@github-actions
Copy link
Copy Markdown

@antekresic, @dbeck: please review this pull request.

Powered by pull-review

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

:PREFIX
SELECT * FROM test_segby ORDER BY time ASC NULLS LAST;

-- Should be optimized
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed copy/paste added by mistake. Those are redundant tests.

@svenklemm
Copy link
Copy Markdown
Member

Please dont abbreviate BatchSortedMerge to BSM in comments and descriptions

@natalya-aksman
Copy link
Copy Markdown
Member Author

natalya-aksman commented May 1, 2026

Please dont abbreviate BatchSortedMerge to BSM in comments and descriptions

Will change to full name.
Changed to BatchSortedMerge in descriptions and comments.

@natalya-aksman natalya-aksman changed the title Allow BSM with no segmentby or sorted on segmentbys pinned to Consts Allow BatchSortedMerge with no segmentby or sorted on segmentbys pinned to Consts May 1, 2026
…ed to Consts, use BatchSortedMerge instead of BSM
* If pathkeys still has items, but we didn't find all segmentby columns,
* we cannot satisfy these pathkeys by sorting the compressed chunk table.
*/
if (i != list_length(pathkeys) &&
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We rule out (i == list_length(pathkeys)) on :3045 above, so this check is redundant.

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.

3 participants