Re: EvalPlanQual behaves oddly for FDW queries involving system columns - Mailing list pgsql-hackers

Hello,

At Tue, 14 Apr 2015 12:10:35 +0900, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote in
<552C852B.2050401@lab.ntt.co.jp>
> On 2015/04/13 23:25, Jim Nasby wrote:
> > On 4/13/15 4:58 AM, Etsuro Fujita wrote:
> >> On 2015/04/10 21:40, Etsuro Fujita wrote:
> >>> On 2015/04/09 12:07, Etsuro Fujita wrote:
> >>>> I'll update the patch, which will contain only an infrastructure for
> >>>> this in the PG core, and will not contain any postgres_fdw change.
> >>>
> >>> I updated the patch based on your comments.  Updated patch attached.  In
> >>> the patch the following FDW APIs have been proposed:
> >>>
> >>> + RowMarkType
> >>> + GetForeignRowMarkType (LockClauseStrength strength);
> >>>
> >>> + bool
> >>> + LockForeignRow (EState *estate,
> >>> +                 ExecRowMark *erm,
> >>> +                 ItemPointer tupleid);
> >>>
> >>> + HeapTuple
> >>> + FetchForeignRow (EState *estate,
> >>> +                  ExecRowMark *erm,
> >>> +                  ItemPointer tupleid);
> >>>
> >>> I think that these APIs allow the FDW that has TIDs to use the rowmark
> >>> options such as ROW_MARK_REFERENCE, ROW_MARK_SHARE and
> >>> ROW_MARK_EXCLUSIVE for its foreign tables so as to match the local
> >>> semantics exactly, for example.
> >>>
> >>> As you mentioned, it would be better to add helper functions to see
> >>> whether the foreign table is referenced by any ExecRowMarks.  ISTM that
> >>> an easy way to do that is to modify ExecFindRowMark() so that it allows
> >>> for the missing case.  I didn't contain such functions in the patch, though.
> >>
> >> I added that function and modified docs a bit.  Please find attached an
> >> updated version of the patch.
> > 
> > Why aren't we allowing SELECT FOR KEY SHARE?
> 
> I omitted that mode (and the FOR NO KEY UPDATE mode) for simplicity, but
> both modes have been allowed.  However, I'm not sure if those modes are
> useful because we don't have information about keys for a remote table.

If I understand this correctly, the two lock modes are no
relation with key column definitions, and are provided as a
weaker lock in exchange for some risks. Like advisory locks. we
can FOR_NO_KEY_UPDATE in the trunsactions that evetually update
"key columns" but also should accept the unexpected result. In
other words, the "key" in the context of "FOR NO KEY UPDATE" and
"FOR KEY SHARE" is only in the programmers' minds.

Apart from feasibility, I don't see no resason to omit them if
this is correct.

As an example, the following operations cause an "unexpected"
result.

Ex. 1
Session A=# create table t (a int primary key, b int);
Session A=# insert into t (select a, 1 from generate_series(0, 99) a);
Session A=# begin;
Session A=# select * from t where a = 1 for no key update;

Session B=# begin;
Session B=# select * from t where a = 1 for key share;
Session B=# update t set a = -a where a = 1;
<session B is blocked>

Ex. 2
Session A=# create table t (a int, b int); -- a is the key in mind.
Session A=# insert into t (select a, 1 from generate_series(0, 99) a);
Session A=# begin;
Session A=# select * from t where a = 1 for no key update;

Session B=# begin;
Session B=# select * from t where a = 1 for key share;

Session A=# update t set a = -a where a = 1;
UPDATE 1
Session A=# commit;

Session B=# select * from t where a = 1;
(0 rows)  -- Woops.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center