Do not sort batches for Batch sorted merge over unordered chunks if query sort matches compressed sort

[natalya-aksman]

When we apply Batch sorted merge to unordered chunks and query sort is matching sort on compressed orderby leading metadata column, we do not need to sort batches before feeding them into Batch sorted merge heap.

--- Should be able to use Batch Sort Merge over unordered chunks 
--- without sorting compressed batches beforehand
SELECT * FROM t_segcol_orderbycol WHERE segcol = 1 ORDER BY orderbycol;

Our batches will already be sorted on (orderby leading metadata column) and the only reason we won't be able to use compressed sort is because the chunk is unordered.

Skipping unnecessary sort for Batch sort merge on unordered chunks can improve performance after Direct compress.

Removing unnecessary sort exposed an existing bug in ColumnarScan exec loop where we didn't quit asking for next batch and then trying to get next batch even after all compressed batches were consumed, as there could still be remaining tuples on the heap.
Sort node hid the bug as it would stop fetching batches residing in a sort buffer, but with the regular column scan we just kept fetching batches as we didn't hit the exit condition.
The bug is now fixed.

Part of work on https://github.com/timescale/eng-database/issues/811

Disable-check: force-changelog-file

[github-actions]

@pnthao, @akuzm: please review this pull request.

Powered by pull-review

[codecov]

Codecov Report

❌ Patch coverage is 88.73239% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tsl/src/nodes/columnar_scan/columnar_scan.c	86.88%	2 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

[natalya-aksman]

LLM Fuzzer found an issue needs addressed: https://github.com/timescale/timescaledb/actions/runs/29857661986

Will add tests for multikey orderby as we can't use compressed sort for BSM for multikey orderby over unordered chunks. UPDATE - done.

[akuzm]

Removing unnecessary sort exposed an existing bug in ColumnarScan exec loop where we didn't quit asking for next batch and then trying to get next batch even after all compressed batches were consumed, as there could still be remaining tuples on the heap.
Sort node hid the bug as it would stop fetching batches residing in a sort buffer, but with the regular column scan we just kept fetching batches as we didn't hit the exit condition.

Oh, I know this problem, sounds like this was actually a violation of Postgres protocol for exec nodes. In Postgres, if ExecProcNode returns NULL once, it means the scan ended, and you're not allowed to call it again. Otherwise some types of nodes will restart the scan.

[antekresic]

Approving but would love to see tests without segmentby to double check index is used correctly in that setup as well.

[natalya-aksman]

Approving but would love to see tests without segmentby to double check index is used correctly in that setup as well.

There is a test on test_nosegby table which checks plan for skipping unnecessary sort, I'll add tests on table with no segmentby and overlapping batches, to check for correctness.