Thread: 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
Attachment
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