Re: Update comments in nodeModifyTable.c

On Wed, Jun 14, 2017 at 10:40 PM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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