Re: Another way to fix inherited UPDATE/DELETE

Hi,

On 2019/02/20 6:48, Tom Lane wrote:
> While contemplating the wreckage of 
> https://commitfest.postgresql.org/22/1778/
> I had the beginnings of an idea of another way to fix that problem.
>
> The issue largely arises from the fact that for UPDATE, we expect
> the plan tree to emit a tuple that's ready to be stored back into
> the target rel ... well, almost, because it also has a CTID or some
> other row-identity column, so we have to do some work on it anyway.
> But the point is this means we potentially need a different
> targetlist for each child table in an inherited UPDATE.
> 
> What if we dropped that idea, and instead defined the plan tree as
> returning only the columns that are updated by SET, plus the row
> identity?  It would then be the ModifyTable node's job to fetch the
> original tuple using the row identity (which it must do anyway) and
> form the new tuple by combining the updated columns from the plan
> output with the non-updated columns from the original tuple.
> 
> DELETE would be even simpler, since it only needs the row identity
> and nothing else.

I had bookmarked link to an archived email of yours from about 5 years
ago, in which you described a similar attack plan for UPDATE planning:

https://www.postgresql.org/message-id/1598.1399826841%40sss.pgh.pa.us

It's been kind of in the back of my mind for a while, even considered
implementing it based on your sketch back then, but didn't have solutions
for some issues surrounding optimization of updates of foreign partitions
(see below).  Maybe I should've mentioned that on this thread at some point.

> Having done that, we could toss inheritance_planner into the oblivion
> it so richly deserves, and just treat all types of inheritance or
> partitioning queries as expand-at-the-bottom, as SELECT has always
> done it.
> 
> Arguably, this would be more efficient even for non-inheritance join
> situations, as less data (typically) would need to propagate through the
> join tree.  I'm not sure exactly how it'd shake out for trivial updates;
> we might be paying for two tuple deconstructions not one, though perhaps
> there's a way to finesse that.  (One easy way would be to stick to the
> old approach when there is no inheritance going on.)
> 
> In the case of a standard inheritance or partition tree, this seems to
> go through really easily, since all the children could share the same
> returned CTID column (I guess you'd also need a TABLEOID column so you
> could figure out which table to direct the update back into).  It gets
> a bit harder if the tree contains some foreign tables, because they might
> have different concepts of row identity, but I'd think in most cases you
> could still combine those into a small number of output columns.

Regarding child target relations that are foreign tables, the
expand-target-inheritance-at-the-bottom approach perhaps leaves no way to
allow pushing the update (possibly with joins) to remote side?

-- no inheritance
explain (costs off, verbose) update ffoo f set a = f.a + 1 from fbar b
where f.a = b.a;
                                              QUERY PLAN

──────────────────────────────────────────────────────────────────────────────────────────────────────
 Update on public.ffoo f
   ->  Foreign Update
         Remote SQL: UPDATE public.foo r1 SET a = (r1.a + 1) FROM
public.bar r2 WHERE ((r1.a = r2.a))
(3 rows)

-- inheritance
explain (costs off, verbose) update p set aa = aa + 1 from ffoo f where
p.aa = f.a;
                                                QUERY PLAN

───────────────────────────────────────────────────────────────────────────────────────────────────────────
 Update on public.p
   Update on public.p1
   Update on public.p2
   Foreign Update on public.p3
   ->  Nested Loop
         Output: (p1.aa + 1), p1.ctid, f.*
         ->  Seq Scan on public.p1
               Output: p1.aa, p1.ctid
         ->  Foreign Scan on public.ffoo f
               Output: f.*, f.a
               Remote SQL: SELECT a FROM public.foo WHERE (($1::integer = a))
   ->  Nested Loop
         Output: (p2.aa + 1), p2.ctid, f.*
         ->  Seq Scan on public.p2
               Output: p2.aa, p2.ctid
         ->  Foreign Scan on public.ffoo f
               Output: f.*, f.a
               Remote SQL: SELECT a FROM public.foo WHERE (($1::integer = a))
   ->  Foreign Update
         Remote SQL: UPDATE public.base3 r5 SET aa = (r5.aa + 1) FROM
public.foo r2 WHERE ((r5.aa = r2.a))
(20 rows)

Does that seem salvageable?

Thanks,
Amit