Fix MULTIEXPR_SUBLINK with partitioned target tables, yet again.

We already tried to fix this in commits 3f7323cbb et al (and follow-on
fixes), but now it emerges that there are still unfixed cases;
moreover, these cases affect all branches not only pre-v14.  I thought
we had eliminated all cases of making multiple clones of an UPDATE's
target list when we nuked inheritance_planner.  But it turns out we
still do that in some partitioned-UPDATE cases, notably including
INSERT ... ON CONFLICT UPDATE, because ExecInitPartitionInfo thinks
it's okay to clone and modify the parent's targetlist.

This fix is based on a suggestion from Andres Freund: let's stop
abusing the ParamExecData.execPlan mechanism, which was only ever
meant to handle initplans, and instead solve the execution timing
problem by having the expression compiler move MULTIEXPR_SUBLINK steps
to the front of their expression step lists.  This is feasible because
(a) all branches still in support compile the entire targetlist of
an UPDATE into a single ExprState, and (b) we know that all
MULTIEXPR_SUBLINKs do need to be evaluated --- none could be buried
inside a CASE, for example.  There is a minor semantics change
concerning the order of execution of the MULTIEXPR's subquery versus
other parts of the parent targetlist, but that seems like something
we can get away with.  By doing that, we no longer need to worry
about whether different clones of a MULTIEXPR_SUBLINK share output
Params; their usage of that data structure won't overlap.

Per bug #17800 from Alexander Lakhin.  Back-patch to all supported
branches.  In v13 and earlier, we can revert 3f7323cbb and follow-on
fixes; however, I chose to keep the SubPlan.subLinkId field added
in ccbb54c72.  We don't need that anymore in the core code, but it's
cheap enough to fill, and removing a plan node field in a minor
release seems like it'd be asking for trouble.

Andres Freund and Tom Lane

Discussion: https://postgr.es/m/17800-ff90866b3906c964@postgresql.org

2 files changed, 148 insertions, 0 deletions

diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 2d49e765de8..e2a0dc80b28 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1713,6 +1713,116 @@ reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
 --
+-- Check handling of MULTIEXPR SubPlans in inherited updates
+--
+create table inhpar(f1 int, f2 name);
+create table inhcld(f2 name, f1 int);
+alter table inhcld inherit inhpar;
+insert into inhpar select x, x::text from generate_series(1,5) x;
+insert into inhcld select x::text, x from generate_series(6,10) x;
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+                               QUERY PLAN                                
+-------------------------------------------------------------------------
+ Update on public.inhpar i
+   Update on public.inhpar i_1
+   Update on public.inhcld i_2
+   ->  Result
+         Output: $2, $3, (SubPlan 1 (returns $2,$3)), i.tableoid, i.ctid
+         ->  Append
+               ->  Seq Scan on public.inhpar i_1
+                     Output: i_1.f1, i_1.f2, i_1.tableoid, i_1.ctid
+               ->  Seq Scan on public.inhcld i_2
+                     Output: i_2.f1, i_2.f2, i_2.tableoid, i_2.ctid
+         SubPlan 1 (returns $2,$3)
+           ->  Limit
+                 Output: (i.f1), (((i.f2)::text || '-'::text))
+                 ->  Seq Scan on public.int4_tbl
+                       Output: i.f1, ((i.f2)::text || '-'::text)
+(15 rows)
+
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+ f1 | f2  
+----+-----
+  1 | 1-
+  2 | 2-
+  3 | 3-
+  4 | 4-
+  5 | 5-
+  6 | 6-
+  7 | 7-
+  8 | 8-
+  9 | 9-
+ 10 | 10-
+(10 rows)
+
+drop table inhpar cascade;
+NOTICE:  drop cascades to table inhcld
+--
+-- And the same for partitioned cases
+--
+create table inhpar(f1 int primary key, f2 name) partition by range (f1);
+create table inhcld1(f2 name, f1 int primary key);
+create table inhcld2(f1 int primary key, f2 name);
+alter table inhpar attach partition inhcld1 for values from (1) to (5);
+alter table inhpar attach partition inhcld2 for values from (5) to (100);
+insert into inhpar select x, x::text from generate_series(1,10) x;
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+                                    QUERY PLAN                                     
+-----------------------------------------------------------------------------------
+ Update on public.inhpar i
+   Update on public.inhcld1 i_1
+   Update on public.inhcld2 i_2
+   ->  Append
+         ->  Seq Scan on public.inhcld1 i_1
+               Output: $2, $3, (SubPlan 1 (returns $2,$3)), i_1.tableoid, i_1.ctid
+               SubPlan 1 (returns $2,$3)
+                 ->  Limit
+                       Output: (i_1.f1), (((i_1.f2)::text || '-'::text))
+                       ->  Seq Scan on public.int4_tbl
+                             Output: i_1.f1, ((i_1.f2)::text || '-'::text)
+         ->  Seq Scan on public.inhcld2 i_2
+               Output: $2, $3, (SubPlan 1 (returns $2,$3)), i_2.tableoid, i_2.ctid
+(13 rows)
+
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+ f1 | f2  
+----+-----
+  1 | 1-
+  2 | 2-
+  3 | 3-
+  4 | 4-
+  5 | 5-
+  6 | 6-
+  7 | 7-
+  8 | 8-
+  9 | 9-
+ 10 | 10-
+(10 rows)
+
+-- Also check ON CONFLICT
+insert into inhpar as i values (3), (7) on conflict (f1)
+  do update set (f1, f2) = (select i.f1, i.f2 || '+');
+select * from inhpar order by f1;  -- tuple order might be unstable here
+ f1 | f2  
+----+-----
+  1 | 1-
+  2 | 2-
+  3 | 3-+
+  4 | 4-
+  5 | 5-
+  6 | 6-
+  7 | 7-+
+  8 | 8-
+  9 | 9-
+ 10 | 10-
+(10 rows)
+
+drop table inhpar cascade;
+--
 -- Check handling of a constant-null CHECK constraint
 --
 create table cnullparent (f1 int);
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 195aedb5ff5..5db6dbc1914 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -630,6 +630,44 @@ reset enable_indexscan;
 reset enable_bitmapscan;
 
 --
+-- Check handling of MULTIEXPR SubPlans in inherited updates
+--
+create table inhpar(f1 int, f2 name);
+create table inhcld(f2 name, f1 int);
+alter table inhcld inherit inhpar;
+insert into inhpar select x, x::text from generate_series(1,5) x;
+insert into inhcld select x::text, x from generate_series(6,10) x;
+
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+
+drop table inhpar cascade;
+
+--
+-- And the same for partitioned cases
+--
+create table inhpar(f1 int primary key, f2 name) partition by range (f1);
+create table inhcld1(f2 name, f1 int primary key);
+create table inhcld2(f1 int primary key, f2 name);
+alter table inhpar attach partition inhcld1 for values from (1) to (5);
+alter table inhpar attach partition inhcld2 for values from (5) to (100);
+insert into inhpar select x, x::text from generate_series(1,10) x;
+
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+
+-- Also check ON CONFLICT
+insert into inhpar as i values (3), (7) on conflict (f1)
+  do update set (f1, f2) = (select i.f1, i.f2 || '+');
+select * from inhpar order by f1;  -- tuple order might be unstable here
+
+drop table inhpar cascade;
+
+--
 -- Check handling of a constant-null CHECK constraint
 --
 create table cnullparent (f1 int);