Обсуждение: 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

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

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