Re: BUG #17800: ON CONFLICT DO UPDATE fails to detect incompatible fields that leads to a server crash - Mailing list pgsql-bugs

I wrote:
> Bisecting produced some interesting results: the "wrong type"
> failure has existed for a pretty long time on HEAD.  The reason
> I don't see it on v13 etc is that commits 3f7323cbb et al fixed
> it in those branches.  That's probably accidental, but I'm too
> tired to poke at it more tonight.

Sigh ... no, it's not accidental: this is exactly the same problem
3f7323cbb fixed, but popping up in two new places.

In the first place, the test cases we had that led to 3f7323cbb
were using traditionally-inherited tables, in which an update
of this sort leads to a plan like

regression=# create table ti (a int, b text);
CREATE TABLE
regression=# create table tichild() inherits(ti);
CREATE TABLE
regression=# explain (verbose, costs off) UPDATE ti SET (a, b) = (SELECT ti.a, ti.b || '+');
                                QUERY PLAN
---------------------------------------------------------------------------
 Update on public.ti
   Update on public.ti ti_1
   Update on public.tichild ti_2
   ->  Result
         Output: $2, $3, (SubPlan 1 (returns $2,$3)), ti.tableoid, ti.ctid
         ->  Append
               ->  Seq Scan on public.ti ti_1
                     Output: ti_1.a, ti_1.b, ti_1.tableoid, ti_1.ctid
               ->  Seq Scan on public.tichild ti_2
                     Output: ti_2.a, ti_2.b, ti_2.tableoid, ti_2.ctid
         SubPlan 1 (returns $2,$3)
           ->  Result
                 Output: ti.a, (ti.b || '+'::text)
(13 rows)

But if the target is a partitioned table, as in Alexander's example:

regression=# explain (verbose, costs off) UPDATE t SET (a, b) = (SELECT t.a, t.b || '+');
                                    QUERY PLAN
-----------------------------------------------------------------------------------
 Update on public.t
   Update on public.tp1 t_1
   Update on public.tp2 t_2
   ->  Append
         ->  Seq Scan on public.tp1 t_1
               Output: $2, $3, (SubPlan 1 (returns $2,$3)), t_1.tableoid, t_1.ctid
               SubPlan 1 (returns $2,$3)
                 ->  Result
                       Output: t_1.a, (t_1.b || '+'::text)
         ->  Seq Scan on public.tp2 t_2
               Output: $2, $3, (SubPlan 1 (returns $2,$3)), t_2.tableoid, t_2.ctid
(11 rows)

I'm not real sure why this discrepancy exists, or whether it's a good
idea.  But at any rate, the reason why I thought 3f7323cbb would only
be needed in the back branches is that I thought we had eliminated all
cases of making multiple clones of an UPDATE's target list when we
nuked inheritance_planner.  But here we have multiple clones of a
MULTIEXPR_SUBLINK SubPlan, and they're all sharing the same output
parameters ($2 and $3, here), and so the confusion about which args list
has to be executed during a recomputation comes right back.

If that were the only problem then I'd seriously think about fixing it
by disallowing this sort of push-down of the UPDATE targetlist when
MULTIEXPR_SUBLINKs are present.  However, the reason we're seeing a
comparable problem in ON CONFLICT is that ExecInitPartitionInfo *also*
makes clones of an UPDATE targetlist, and it is far too naive to fix
this problem.

Not sure about a good fix.  We could imagine porting v13's
SS_make_multiexprs_unique logic forward into the newer branches,
but that depends on a lot of planner infrastructure that won't
be available when ExecInitPartitionInfo runs.  I think in the
back branches we might be forced into disallowing MULTIEXPR_SUBLINKs
in ON CONFLICT targetlists, because supporting them properly is
looking like a mess.

At this point I'm seriously regretting the entire MULTIEXPR_SUBLINK
design, and am wondering if we could find a different solution for
that.  That couldn't lead to a back-patchable fix, obviously.

Another angle is that having ExecInitPartitionInfo doing this sort
of work is a big misallocation of responsibility.  If these tlists
were getting built in the planner it'd be far easier to fix.

            regards, tom lane