Copy-pasto in the ExecForeignDelete documentation
Hi,
While working on FDW DML pushdown, I ran into a copy-pasto in the
ExecForeignDelete documentation in fdwhandler.sgml:
The data in the returned slot is used only if the <command>DELETE</>
query has a <literal>RETURNING</> clause or the foreign table has
an <literal>AFTER ROW</> trigger. Triggers require all columns,
but the
I don't think the data is referenced by the AFTER ROW DELETE triggers.
Attached is a patch to fix that. The patch also avoids adding an
unnecessary RETURNING clause to DELETE when deparsing a remote DELETE
statement in postgres_fdw.
I'll add this to the next CF.
Best regards,
Etsuro Fujita
Attachments:
ExecForeignDelete.patchapplication/x-patch; name=ExecForeignDelete.patchDownload
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 1078,1085 **** deparseDeleteSql(StringInfo buf, PlannerInfo *root,
deparseRelation(buf, rel);
appendStringInfoString(buf, " WHERE ctid = $1");
! deparseReturningList(buf, root, rtindex, rel,
! rel->trigdesc && rel->trigdesc->trig_delete_after_row,
returningList, retrieved_attrs);
}
--- 1078,1085 ----
deparseRelation(buf, rel);
appendStringInfoString(buf, " WHERE ctid = $1");
! /* No need to retrieve columns for AFTER ROW DELETE triggers */
! deparseReturningList(buf, root, rtindex, rel, false,
returningList, retrieved_attrs);
}
*** a/doc/src/sgml/fdwhandler.sgml
--- b/doc/src/sgml/fdwhandler.sgml
***************
*** 606,613 **** ExecForeignDelete (EState *estate,
<para>
The data in the returned slot is used only if the <command>DELETE</>
! query has a <literal>RETURNING</> clause or the foreign table has
! an <literal>AFTER ROW</> trigger. Triggers require all columns, but the
FDW could choose to optimize away returning some or all columns depending
on the contents of the <literal>RETURNING</> clause. Regardless, some
slot must be returned to indicate success, or the query's reported row
--- 606,614 ----
<para>
The data in the returned slot is used only if the <command>DELETE</>
! query has a <literal>RETURNING</> clause. (Note that the data is not
! referenced by <literal>AFTER ROW</> triggers on the foreign table in
! the <command>DELETE</> case.) The
FDW could choose to optimize away returning some or all columns depending
on the contents of the <literal>RETURNING</> clause. Regardless, some
slot must be returned to indicate success, or the query's reported row
On Mon, Feb 1, 2016 at 5:26 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
I don't think the data is referenced by the AFTER ROW DELETE triggers.
Why do you think that? And why would DELETE triggers be different
from UPDATE triggers, which do something similar?
I looked up the history of this code and it was introduced in
7cbe57c3, which added support for triggers on foreign tables. Noah
did that commit and he's rarely wrong about stuff like this, so I
suspect you may be missing something. One thing to consider is
whether the version of the row that finally gets deleted is
necessarily the same as the version originally selected from the
remote side; e.g. suppose the remote side has triggers, too.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016/02/04 0:13, Robert Haas wrote:
On Mon, Feb 1, 2016 at 5:26 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:I don't think the data is referenced by the AFTER ROW DELETE triggers.
Why do you think that? And why would DELETE triggers be different
from UPDATE triggers, which do something similar?
As for the UPDATE case, I think local AFTER ROW UPDATE triggers have to
reference the data since a BEFORE trigger on the remote server might
change the to-be-updated version of the row originally assigned. But as
for the DELETE case, I was not thinking so.
I looked up the history of this code and it was introduced in
7cbe57c3, which added support for triggers on foreign tables. Noah
did that commit and he's rarely wrong about stuff like this, so I
suspect you may be missing something. One thing to consider is
whether the version of the row that finally gets deleted is
necessarily the same as the version originally selected from the
remote side; e.g. suppose the remote side has triggers, too.
Maybe I'm missing something, but I was thinking that version should be
the same as the version originally selected from the remote server; the
delete would be otherwise discarded since the updated version would not
satisfy the delete's condition, something similar to "ctid = $1" in the
postgres_fdw case, during an EvalPlanQual-like recheck on the remote server.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers