Kohei KaiGai wrote:
>> Weird, that fails for me.
> Both of the troubles you reported was fixed with attached patch.
> I also added relevant test cases into regression test, please check it.
It passes the regression tests, and solves the problems I found.
I came up with one more query that causes a problem:
CREATE TABLE test(
id integer PRIMARY KEY,
val text NOT NULL
);
INSERT INTO test(id, val) VALUES (1, 'one');
CREATE FOREIGN TABLE rtest(
id integer not null,
val text not null
) SERVER loopback OPTIONS (relname 'test');
/* loopback points to the same database */
WITH ch AS (
UPDATE test
SET val='changed'
RETURNING id
) UPDATE rtest
SET val='new'
FROM ch
WHERE rtest.id = ch.id;
This causes a deadlock, but one that is not detected;
the query just keeps hanging.
The UPDATE in the CTE has the rows locked, so the
SELECT ... FOR UPDATE issued via the FDW connection will hang
indefinitely.
I wonder if that's just a pathological corner case that we shouldn't
worry about. Loopback connections for FDWs themselves might not
be so rare, for example as a substitute for autonomous subtransactions.
I guess it is not easily possible to detect such a situation or
to do something reasonable about it.
>> I took a brief look at the documentation; that will need some more
>> work.
>
> Yep, I believe it should be revised prior to this patch being committed.
I tried to overhaul the documentation, see the attached patch.
There was one thing that I was not certain of:
You say that for writable foreign tables, BeginForeignModify
and EndForeignModify *must* be implemented.
I thought that these were optional, and if you can do your work
with just, say, ExecForeignDelete, you could do that.
I left that paragraph roughly as it is, please change it if
appropriate.
I also changed the misspelled "resultRelaion" and updated a
few comments.
May I suggest to split the patch in two parts, one for
all the parts that affect postgres_fdw and one for the rest?
That might make the committer's work easier, since
postgres_fdw is not applied (yet).
Yours,
Laurenz Albe