Skip to content

Check MergeAppend node in share input mutator #1204

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

Merged
merged 3 commits into from
Jul 2, 2025

Conversation

reshke
Copy link
Contributor

@reshke reshke commented Jul 1, 2025

Do recursive call in plan walker for MergeAppend type of plan.

When planner desides to merge two sorted subplans one of which has Share Input Scan node, executer fails to execute this, becuase of wrogly aligned internals structures.

In turn out, it was forgotten in scareinput tree walker to do proper recursion call

repro:


reshke=# explain
with inp as MATERIALIZED (select * from  tt4 ) select a,b,c, count(distinct d), count(distinct e), count(distinct f) from inp group by 1,2,3
union all
select a,b,c, count(distinct d), count(distinct e), count(distinct f) from tt4 group by 1,2,3
order by 1;
                                           QUERY PLAN
-------------------------------------------------------------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)  (cost=1098.77..8775.44 rows=11000 width=36)
   Merge Key: tt4.a
   ->  Merge Append  (cost=1098.77..1442.11 rows=3667 width=36)
         Sort Key: tt4.a
         ->  GroupAggregate  (cost=864.39..1042.72 rows=333 width=36)
               Group Key: tt4.a, tt4.b, tt4.c
               ->  Sort  (cost=864.39..889.39 rows=10000 width=24)
                     Sort Key: tt4.a, tt4.b, tt4.c
                     ->  Shared Scan (share slice:id 1:0)  (cost=39.33..0.00 rows=3333 width=24)
                           ->  Seq Scan on tt4  (cost=0.00..39.33 rows=3333 width=24)
         ->  GroupAggregate  (cost=234.38..326.05 rows=3333 width=36)
               Group Key: tt4_1.a, tt4_1.b, tt4_1.c
               ->  Sort  (cost=234.38..242.71 rows=3333 width=24)
                     Sort Key: tt4_1.a, tt4_1.b, tt4_1.c
                     ->  Seq Scan on tt4 tt4_1  (cost=0.00..39.33 rows=3333 width=24)
 Optimizer: Postgres query optimizer
(16 rows)

reshke=# explain analyze
with inp as MATERIALIZED (select * from  tt4 ) select a,b,c, count(distinct d), count(distinct e), count(distinct f) from inp group by 1,2,3
union all
select a,b,c, count(distinct d), count(distinct e), count(distinct f) from tt4 group by 1,2,3
order by 1;
ERROR:  cannot execute alien Share Input Scan (nodeShareInputScan.c:364)  (seg1 slice1 127.0.1.1:7003 pid=17684) (nodeShareInputScan.c:364)
reshke=#

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


Copy link
Contributor

@avamingli avamingli left a comment

Choose a reason for hiding this comment

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

Good Catch!
It would be better to add a test.

@avamingli
Copy link
Contributor

Found that there is a Merge commit(using rebase to update with the main branch can help avoid this), remember to squash the commits when merging this PR.

@reshke
Copy link
Contributor Author

reshke commented Jul 2, 2025

Good Catch! It would be better to add a test.

sure! Which regression test file fits better?

@avamingli
Copy link
Contributor

Good Catch! It would be better to add a test.

sure! Which regression test file fits better?

Anyone related to cte is ok as shared scan is with cte.

@reshke reshke force-pushed the jiihuuhuijuhgyftu branch from 44daf0b to e3c6f11 Compare July 2, 2025 08:51
Do recursive call in plan walker for MergeAppend type of plan.

When planner desides to merge two sorted subplans one
of which has Share Input Scan node, executer fails to execute this,
because of wrogly aligned internal structures.

It turns out, it was forgotten to do proper recursion call
in shareinput tree walker
@reshke reshke force-pushed the jiihuuhuijuhgyftu branch from e3c6f11 to 5eaa3f2 Compare July 2, 2025 09:25
@reshke
Copy link
Contributor Author

reshke commented Jul 2, 2025

Added test to with_query regression suite

@reshke reshke merged commit f5dcd91 into apache:main Jul 2, 2025
41 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants