Fix wrong result in parallel ChunkAppend#9388
Draft
akuzm wants to merge 6 commits intotimescale:mainfrom
Draft
Conversation
Some fields were not initialized properly in case when there's no startup exclusion but runtime parent exclusion, which led us to excluding the chunks that actually match. Refactor the way we work with the lists of the matched plans to prevent confusion. Now we use the same initial list of matched plans always, and keep a separate list of plan states (excluded/finished/etc) which is used for iteration.
The test for parallel ChunkAppend with InitPlan params filtered on column i (the partitioning column). This caused the planner to set runtime_exclusion_children, which made it pick a Single Copy Gather plan instead of a parallel-aware ChunkAppend. The shared-memory subplan coordination path was never exercised, so the test passed even with the buggy code. Switch the filter to column j (non-partitioning column) so the planner produces a truly parallel ChunkAppend with only runtime_exclusion_parent. This correctly exposes the bug where parallel workers received empty subplan lists from shared memory. Verified on PG17 and PG18: - Old code + new test: workers_ok = f (bug detected) - Fixed code + new test: workers_ok = t (bug fixed)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The partial and non-partial plan tests only ran EXPLAIN without executing the query, so the worker code path that marks non-partial plans as finished was never covered. Add actual query execution to exercise that path.
Cover the do_runtime_exclusion code path where ts_chunk_append_get_scan_plan returns NULL for a MergeAppend child in an ordered ChunkAppend with space partitioning. This exercises the continue at exec.c:456 (scan == NULL skip). The test creates a space-partitioned hypertable where one time slice has data in both space partitions (producing a MergeAppend child) and another has data in only one partition (producing a direct scan child). A LATERAL join triggers runtime chunk exclusion, which must handle both child types.
Shared tests run in parallel in the same database and must not create new tables. Move to tsl/test/sql/ where each test gets its own database.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some fields were not initialized properly in case when there's no startup exclusion but runtime parent exclusion, which led us to excluding the chunks that actually match.
Refactor the way we work with the lists of the matched plans to prevent confusion. Now we use the same initial list of matched plans always, and keep a separate list of plan states (excluded/finished/etc) which is used for iteration.