Re: [HACKERS] Update comments in nodeModifyTable.c - Mailing list pgsql-hackers

From Robert Haas
Subject Re: [HACKERS] Update comments in nodeModifyTable.c
Date
Msg-id CA+Tgmob0NcvuYyhNC0M27DCQZXxd5TbURXF09nxAtBwCtCZp=Q@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Update comments in nodeModifyTable.c  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Responses Re: [HACKERS] Update comments in nodeModifyTable.c
List pgsql-hackers
On Wed, Jun 14, 2017 at 10:40 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> Agreed, but I think it's better to add that detail to this comment in
> ExecInitModifyTable:
>
>      * Initialize the junk filter(s) if needed.  INSERT queries need a
> filter
>      * if there are any junk attrs in the tlist.  UPDATE and DELETE always
>      * need a filter, since there's always a junk 'ctid' or 'wholerow'
>      * attribute present --- no need to look first.
>
> I'd also like to propose to update the third sentence in this comment, since
> there isn't necessarily a ctid or wholerow in the UPDATE/DELETE tlist when
> the result relation is a foreign table.
>
> Attached is an updated version of the patch.

Well, now I'm confused:
     * Initialize the junk filter(s) if needed.  INSERT queries need a filter     * if there are any junk attrs in the
tlist. UPDATE and DELETE always     * need a filter, since there's always a junk 'ctid' or 'wholerow'
 
-     * attribute present --- no need to look first.
+     * attribute present if not foreign table, and if foreign table, there
+     * are always junk attributes present the FDW needs to identify the exact
+     * row to update or delete --- no need to look first.  For foreign tables,
+     * there's also a wholerow attribute when the relation has a row-level
+     * trigger on UPDATE/DELETE but not on INSERT.

So the first part of the change weakens the assertion that a 'ctid' or
'wholerow' attribute will always be present by saying that an FDW may
instead have other attributes sufficient to identify the row.  But
then the additional sentence says that there will be a 'wholerow'
attribute after all.  So this whole change seems to me to be going
around in a circle.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Jeevan Ladhe
Date:
Subject: Re: [HACKERS] Default Partition for Range
Next
From: Andrew Dunstan
Date:
Subject: Re: [HACKERS] Testlib.pm vs msys