Re: Problem while updating a foreign table pointing to apartitioned table on foreign server

Thanks for the discussion.

At Thu, 7 Jun 2018 19:16:57 +0530, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote in
<CAFjFpRd+Bz-DwpnwsY_3uFkALmQgDRTdp_DKhxgm1H20dXs=ow@mail.gmail.com>
> On Tue, Jun 5, 2018 at 3:40 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> > Hello.
> >
> > At Mon, 04 Jun 2018 20:58:28 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote
in<20180604.205828.208262556.horiguchi.kyotaro@lab.ntt.co.jp>
 
> >> It fails on some join-pushdown cases since it doesn't add tid
> >> columns to join tlist.  I suppose that build_tlist_to_deparse
> >> needs something but I'll consider further tomorrow.
> >
> > I made it work with a few exceptions and bumped.  PARAM_EXEC
> > doesn't work at all in a case where Sort exists between
> > ForeignUpdate and ForeignScan.
> >
> > =====
> > explain (verbose, costs off)
> > update bar set f2 = f2 + 100
> > from
> >   ( select f1 from foo union all select f1+3 from foo ) ss
> > where bar.f1 = ss.f1;
> >                                   QUERY PLAN
> > -----------------------------------------------------------------------------
> >  Update on public.bar
> >    Update on public.bar
> >    Foreign Update on public.bar2
> >      Remote SQL: UPDATE public.loct2 SET f2 = $3 WHERE tableoid = $1 AND ctid = $2
> > ...
> >    ->  Merge Join
> >          Output: bar2.f1, (bar2.f2 + 100), bar2.f3, (ROW(foo.f1))
> >          Merge Cond: (bar2.f1 = foo.f1)
> >          ->  Sort
> >                Output: bar2.f1, bar2.f2, bar2.f3, bar2.tableoid, bar2.ctid
> >                Sort Key: bar2.f1
> >                ->  Foreign Scan on public.bar2
> >                      Output: bar2.f1, bar2.f2, bar2.f3, bar2.tableoid, bar2.ctid
> >                      Remote SQL: SELECT f1, f2, f3, ctid, tableoid FROM public.loct2 FOR UPDATE
> > =====
> 
> What's the problem that you faced?

The required condtion for PARAM_EXEC to work is that executor
ensures the correspondence between the setter the reader of a
param like ExecNestLoop is doing. The Sort node breaks the
correspondence between the tuple obtained from the Foreign Scan
and that ForeignUpdate is updating. Specifically Foreign Update
upadtes the first tuple using the tableoid for the last tuple
from the Foreign Scan.

> > Even if this worked fine, it cannot be back-patched.  We need an
> > extra storage moves together with tuples or prevent sorts or
> > something like from being inserted there.
> 
> I think your approach still has the same problem that it's abusing the
> tableOid field in the heap tuple to store tableoid from the remote as
> well as local table. That's what Robert and Tom objected to [1], [2]

It's wrong understanding. PARAM_EXEC conveys remote tableoids
outside tuples and each tuple is storing correct (= local)
tableoid.

> > At Fri, 1 Jun 2018 10:21:39 -0400, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote in
<CAFjFpRdraYcQnD4tKzNuP1uP6L-gnizi4HLU_UA=28Q2M4zoDA@mail.gmail.com>
> >> I am not suggesting to commit 0003 in my patch set, but just 0001 and
> >> 0002 which just raise an error when multiple rows get updated when
> >> only one row is expected to be updated.
> >
> > So I agree to commit the two at least in order to prevent doing
> > wrong silently.
> 
> I haven't heard any committer's opinion on this one yet.
> 
> [1] https://www.postgresql.org/message-id/CA+TgmobUHHZiDR=HCU4n30yi9_PE175itTbFK6T8JxzwkRAWAg@mail.gmail.com
> [2] https://www.postgresql.org/message-id/7936.1526590932%40sss.pgh.pa.us

Agreed. We need any comment to proceed.

I have demonstrated and actually shown a problem of the
PARAM_EXEC case. (It seems a bit silly that I actually found the
problem after it became almost workable, though..)  If tuples
were not copied we will be able to use the address to identify a
tuple but actually they are. (Anyway we soudn't do that.)

A. Just detecting and reporting/erroring the problematic case.

B. Giving to Sort-like nodes an ability to convert PARAMS into
   junk columns.

C. Adding a space for 64bit tuple identifier in a tuple header.

D. Somehow inhibiting tuple-storing node like Sort between. (This
  should break something working.)


B seems to have possibility to fix this but I haven't have a
concrete design of it. With C, I see 2 bits of room in infomask2
and we can use one of the bits to indicate that the tuple has an
extra 64-bit tuple identifier. It could be propagated to desired
place but I'm not sure it is in acceptable shape.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center