On 2015/05/12 7:42, Tom Lane wrote:
> On further consideration ...
Thanks for the consideration!
> I don't much like the division of labor between LockForeignRow and
> FetchForeignRow. In principle that would lead to not one but two
> extra remote accesses per locked row in SELECT FOR UPDATE, at least
> in the case that an EvalPlanQual recheck is required. (I see that
> in your prototype patch for postgres_fdw you attempt to avoid that
> by saving a retrieved tuple in LockForeignRow and then returning it
> in FetchForeignRow, but that seems extremely ugly and bug-prone,
> since there is nothing in the API spec insisting that those calls match
> up one-to-one.) The fact that locking and fetching a tuple are separate
> operations in the heapam API is a historical artifact that probably
> doesn't make sense to duplicate in the FDW API.
I understand your concern about the postgres_fdw patch. However, I
think we should divide this into the two routines as the core patch
does, because I think that would allow an FDW author to implement these
routines so as to improve the efficiency for scenarios that seldom need
fetching, by not retrieving and saving locked tuples in LockForeignRow.
> Another problem is that we fail to detect whether an EvalPlanQual recheck
> is required during a SELECT FOR UPDATE on a remote table, which we
> certainly ought to do if the objective is to make postgres_fdw semantics
> match the local ones.
I think that is interesting in theory, but I'm not sure that is worth
much in practice.
Best regards,
Etsuro Fujita