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

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: robertmhaas(at)gmail(dot)com
Cc: ashutosh(dot)bapat(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Problem while updating a foreign table pointing to a partitioned table on foreign server
Date: 2018-05-22 01:28:14
Message-ID: 20180522.102814.170219446.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 18 May 2018 15:31:07 -0400, Robert Haas <robertmhaas(at)gmail(dot)com> wrote in <CA+TgmoaBuzhhcA21sAm7wH+A-GH2d6GkKhVapkqhnHOW85dDXg(at)mail(dot)gmail(dot)com>
> On Fri, May 18, 2018 at 4:29 AM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > I have reached to the same thought.
> >
> > The point here is that it is a base relation, which is not
> > assumed to have additional columns not in its definition,
> > including nonsystem junk columns. I'm not sure but it seems not
> > that simple to give base relations an ability to have junk
> > columns.
>
> Do you know where that assumption is embedded specifically?

Taking the question literally, I see that add_vars_to_targetlist
accepts neither nonsystem (including whole row vars) junk columns
nor nonjunk columns that is not defined in the base relation. The
first line of the following code is that.

> Assert(attno >= rel->min_attr && attno <= rel->max_attr);
> attno -= rel->min_attr;
> if (rel->attr_needed[attno] == NULL)

In the last line attr_needed is of an array of (max_attr -
min_attr) elements, which is allocated in get_relation_info. I
didn't go further so it might be easier than I'm thinking but
anyway core-side modification (seems to me) is required at any
rate.

> If you're correct, then the FDW API is and always has been broken by
> design for any remote data source that uses a row identifier other
> than CTID, unless every foreign table definition always includes the
> row identifier as an explicit column.

I actually see that. Oracle-FDW needs to compose row
identification by specifying "key" column option in relation
definition and the key columns are added as resjunk column. This
is the third (or, forth?) option of my comment upthread that was
said as "not only bothersome".

https://github.com/laurenz/oracle_fdw

| Column options (from PostgreSQL 9.2 on)
| key (optional, defaults to "false")
|
| If set to yes/on/true, the corresponding column on the foreign
| Oracle table is considered a primary key column. For UPDATE and
| DELETE to work, you must set this option on all columns that
| belong to the table's primary key.

> I might be wrong here, but I'm
> pretty sure Tom wouldn't have committed this API in the first place
> with such a glaring hole in the design.

I see the API is still not broken in a sense, the ctid of
postgres_fdw is necessarily that of remote table. If we have a
reasonable mapping between remote tableoid:ctid and local ctid,
it works as expected. But such mapping seems to be rather
difficult to create since I don't find a generic way wihtout
needing auxiliary information, and at least there's no guarantee
that ctid has enough space for rows from multiple tables.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2018-05-22 01:30:44 Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager
Previous Message Andres Freund 2018-05-22 01:08:23 Re: Postgres, fsync, and OSs (specifically linux)