On 2017/06/07 0:30, Robert Haas wrote:
On Mon, Jun 5, 2017 at 4:45 AM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
While working on [1], I noticed that the comment in ExecModifyTable:
* Foreign table updates have a wholerow attribute when the
* relation has an AFTER ROW trigger.
is not 100% correct because a foreign table has a wholerow attrubute when
the relation has an AFTER ROW or BEFORE ROW trigger (see
rewriteTargetListUD). So I'd propose s/an AFTER ROW trigger/a row-level
trigger/. Attached is a patch for that.
That seems better, but looking at rewriteTargetListUD, it seems that
the actual rule is that this happens when there is a row-level on
either UPDATE or DELETE. If there is only a row-level trigger on
INSERT, then it is not done. Perhaps we should try to include that
detail in the comment as well.
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.
Best regards,
Etsuro Fujita
diff --git a/src/backend/executor/nodeModifyTable.c
b/src/backend/executor/nodeModifyTable.c
index cf555fe..07bc870 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1554,7 +1554,7 @@ ExecModifyTable(ModifyTableState *node)
* the old relation tuple.
*
* Foreign table updates have a wholerow
attribute when the
- * relation has an AFTER ROW trigger. Note
that the wholerow
+ * relation has a row-level trigger. Note that
the wholerow
* attribute does not carry system columns.
Foreign table
* triggers miss seeing those, except that we
know enough here
* to set t_tableOid. Quite separately from
this, the FDW may
@@ -2025,7 +2025,11 @@ ExecInitModifyTable(ModifyTable *node, EState *estate,
int eflags)
* 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.
*
* If there are multiple result relations, each one needs its own junk
* filter. Note multiple rels are only possible for UPDATE/DELETE, so
we
@@ -2093,7 +2097,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate,
int eflags)
else if (relkind ==
RELKIND_FOREIGN_TABLE)
{
/*
- * When there is an AFTER
trigger, there should be a
+ * When there is a row-level
trigger, there should be a
* wholerow attribute.
*/
j->jf_junkAttNo =
ExecFindJunkAttribute(j, "wholerow");
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
