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. Best regards, Etsuro Fujita
*** a/doc/src/sgml/fdwhandler.sgml --- b/doc/src/sgml/fdwhandler.sgml *************** *** 211,216 **** BeginForeignScan (ForeignScanState *node, --- 211,230 ---- </para> <para> + If the FDW supports <command>SELECT FOR UPDATE/SHARE</> row locking, + this routine should perform any initialization needed prior to the + actual row locking if the foreign table is referenced in a + <command>SELECT FOR UPDATE/SHARE</>. To decide whether the foreign + table is referenced in a <command>SELECT FOR UPDATE/SHARE</> or not, + it's recommended to use <function>ExecFindRowMark</> with the missing_ok + argument allowing a NULL return if the structure is not. Information + about the row locking is accessible through its return value + (<structname>ExecRowMark</> struct). (The <structfield>fdw_state</> + field of <structname>ExecRowMark</> is available for the FDW to store + any private state it needs for the operation.) + </para> + + <para> Note that when <literal>(eflags & EXEC_FLAG_EXPLAIN_ONLY)</> is true, this function should not perform any externally-visible actions; it should only do the minimum required to make the node state valid *************** *** 598,603 **** IsForeignRelUpdatable (Relation rel); --- 612,699 ---- </sect2> + <sect2 id="fdw-callbacks-row-locking"> + <title>FDW Routines For <command>SELECT FOR UPDATE/SHARE</> row locking + </title> + + <para> + If an FDW supports <command>SELECT FOR UPDATE/SHARE</> row locking, + it should provide the following callback functions: + </para> + + <para> + <programlisting> + RowMarkType + GetForeignRowMarkType (LockClauseStrength strength); + </programlisting> + + Report which row-marking option to support for a lock strength + associated with a <command>SELECT FOR UPDATE/SHARE</> request. + This is called at the beginning of query planning. + <literal>strength</> is a member of the <literal>LockClauseStrength</> + enum type. + The return value should be a member of the <literal>RowMarkType</> + enum type. See + <filename>src/include/nodes/lockoptions.h</> and + <filename>src/include/nodes/plannodes.h</> for information about + these enum types. + </para> + + <para> + If the <function>GetForeignRowMarkType</> pointer is set to + <literal>NULL</>, the default option is selected for any lock strength, + and both <function>LockForeignRow</> and <function>FetchForeignRow</> + described below will not be called at query execution time. + </para> + + <para> + <programlisting> + bool + LockForeignRow (EState *estate, + ExecRowMark *erm, + ItemPointer tupleid); + </programlisting> + + Lock one tuple in the foreign table. + <literal>estate</> is global execution state for the query. + <literal>erm</> is the <structname>ExecRowMark</> struct describing + the target foreign table. + <literal>tupleid</> identifies the tuple to be locked. + This function should return <literal>true</>, if the FDW lock the tuple + successfully. Otherwise, return <literal>false</>. + </para> + + <para> + If the <function>LockForeignRow</> pointer is set to + <literal>NULL</>, attempts to lock rows will fail + with an error message. + </para> + + <para> + <programlisting> + HeapTuple + FetchForeignRow (EState *estate, + ExecRowMark *erm, + ItemPointer tupleid); + </programlisting> + + Fetch one tuple from the foreign table. + <literal>estate</> is global execution state for the query. + <literal>erm</> is the <structname>ExecRowMark</> struct describing + the target foreign table. + <literal>tupleid</> identifies the tuple to be fetched. + This function should return the fetched tuple, if the FDW fetch the + tuple successfully. Otherwise, return NULL. + </para> + + <para> + If the <function>FetchForeignRow</> pointer is set to + <literal>NULL</>, attempts to lock rows will fail + with an error message. + </para> + + </sect2> + <sect2 id="fdw-callbacks-explain"> <title>FDW Routines for <command>EXPLAIN</></title> *************** *** 1011,1017 **** GetForeignServerByName(const char *name, bool missing_ok); join conditions. However, matching the local semantics exactly would require an additional remote access for every row, and might be impossible anyway depending on what locking semantics the external data ! source provides. </para> </sect1> --- 1107,1116 ---- join conditions. However, matching the local semantics exactly would require an additional remote access for every row, and might be impossible anyway depending on what locking semantics the external data ! source provides. If necessary, however, the FDW can change this behavior ! so as to match the local semantics, using its callback functions ! <function>GetForeignRowMarkType</>, <function>LockForeignRow</>, and ! <function>FetchForeignRow</>. </para> </sect1> *** a/src/backend/executor/execMain.c --- b/src/backend/executor/execMain.c *************** *** 855,860 **** InitPlan(QueryDesc *queryDesc, int eflags) --- 855,861 ---- erm->markType = rc->markType; erm->waitPolicy = rc->waitPolicy; ItemPointerSetInvalid(&(erm->curCtid)); + erm->fdw_state = NULL; estate->es_rowMarks = lappend(estate->es_rowMarks, erm); } *************** *** 1098,1103 **** CheckValidResultRel(Relation resultRel, CmdType operation) --- 1099,1106 ---- static void CheckValidRowMarkRel(Relation rel, RowMarkType markType) { + FdwRoutine *fdwroutine; + switch (rel->rd_rel->relkind) { case RELKIND_RELATION: *************** *** 1133,1143 **** CheckValidRowMarkRel(Relation rel, RowMarkType markType) RelationGetRelationName(rel)))); break; case RELKIND_FOREIGN_TABLE: ! /* Should not get here; planner should have used ROW_MARK_COPY */ ! ereport(ERROR, ! (errcode(ERRCODE_WRONG_OBJECT_TYPE), ! errmsg("cannot lock rows in foreign table \"%s\"", ! RelationGetRelationName(rel)))); break; default: ereport(ERROR, --- 1136,1150 ---- RelationGetRelationName(rel)))); break; case RELKIND_FOREIGN_TABLE: ! /* Okay only if the FDW supports it */ ! fdwroutine = GetFdwRoutineForRelation(rel, false); ! if ((markType != ROW_MARK_REFERENCE && ! fdwroutine->LockForeignRow == NULL) || ! fdwroutine->FetchForeignRow == NULL) ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("cannot lock rows in foreign table \"%s\"", ! RelationGetRelationName(rel)))); break; default: ereport(ERROR, *************** *** 1886,1892 **** ExecBuildSlotValueDescription(Oid reloid, * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index */ ExecRowMark * ! ExecFindRowMark(EState *estate, Index rti) { ListCell *lc; --- 1893,1899 ---- * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index */ ExecRowMark * ! ExecFindRowMark(EState *estate, Index rti, bool missing_ok) { ListCell *lc; *************** *** 1897,1903 **** ExecFindRowMark(EState *estate, Index rti) if (erm->rti == rti) return erm; } ! elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti); return NULL; /* keep compiler quiet */ } --- 1904,1911 ---- if (erm->rti == rti) return erm; } ! if (!missing_ok) ! elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti); return NULL; /* keep compiler quiet */ } *************** *** 2401,2407 **** EvalPlanQualFetchRowMarks(EPQState *epqstate) if (erm->markType == ROW_MARK_REFERENCE) { ! Buffer buffer; Assert(erm->relation != NULL); --- 2409,2415 ---- if (erm->markType == ROW_MARK_REFERENCE) { ! HeapTuple copyTuple; Assert(erm->relation != NULL); *************** *** 2415,2428 **** EvalPlanQualFetchRowMarks(EPQState *epqstate) tuple.t_self = *((ItemPointer) DatumGetPointer(datum)); /* okay, fetch the tuple */ - if (!heap_fetch(erm->relation, SnapshotAny, &tuple, &buffer, - false, NULL)) - elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); ! /* successful, copy and store tuple */ ! EvalPlanQualSetTuple(epqstate, erm->rti, ! heap_copytuple(&tuple)); ! ReleaseBuffer(buffer); } else { --- 2423,2455 ---- tuple.t_self = *((ItemPointer) DatumGetPointer(datum)); /* okay, fetch the tuple */ ! if (erm->relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE) ! { ! FdwRoutine *fdwroutine; ! ! /* let the FDW do the work */ ! fdwroutine = GetFdwRoutineForRelation(erm->relation, false); ! copyTuple = fdwroutine->FetchForeignRow(epqstate->estate, ! erm, &tuple.t_self); ! if (copyTuple == NULL) ! elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); ! } ! else ! { ! Buffer buffer; ! ! if (!heap_fetch(erm->relation, SnapshotAny, &tuple, &buffer, ! false, NULL)) ! elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); ! ! /* successful, copy tuple */ ! copyTuple = heap_copytuple(&tuple); ! ReleaseBuffer(buffer); ! } ! ! /* store tuple */ ! EvalPlanQualSetTuple(epqstate, erm->rti, copyTuple); } else { *** a/src/backend/executor/nodeLockRows.c --- b/src/backend/executor/nodeLockRows.c *************** *** 25,30 **** --- 25,31 ---- #include "access/xact.h" #include "executor/executor.h" #include "executor/nodeLockRows.h" + #include "foreign/fdwapi.h" #include "storage/bufmgr.h" #include "utils/rel.h" #include "utils/tqual.h" *************** *** 112,117 **** lnext: --- 113,141 ---- tuple.t_self = *((ItemPointer) DatumGetPointer(datum)); /* okay, try to lock the tuple */ + + if (erm->relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE) + { + FdwRoutine *fdwroutine; + + /* Let the FDW do the work */ + fdwroutine = GetFdwRoutineForRelation(erm->relation, false); + if (!fdwroutine->LockForeignRow(estate, erm, &tuple.t_self)) + { + /* couldn't get the lock */ + goto lnext; + } + /* got the lock successfully */ + + /* + * Remember locked tuple's TID for EvalPlanQual testing, not for + * WHERE CURRENT OF, which is not supported for foreign tables. + */ + erm->curCtid = tuple.t_self; + + continue; + } + switch (erm->markType) { case ROW_MARK_EXCLUSIVE: *************** *** 253,259 **** lnext: ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(lc); ExecRowMark *erm = aerm->rowmark; HeapTupleData tuple; ! Buffer buffer; /* ignore non-active child tables */ if (!ItemPointerIsValid(&(erm->curCtid))) --- 277,283 ---- ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(lc); ExecRowMark *erm = aerm->rowmark; HeapTupleData tuple; ! HeapTuple copyTuple; /* ignore non-active child tables */ if (!ItemPointerIsValid(&(erm->curCtid))) *************** *** 267,280 **** lnext: /* okay, fetch the tuple */ tuple.t_self = erm->curCtid; - if (!heap_fetch(erm->relation, SnapshotAny, &tuple, &buffer, - false, NULL)) - elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); ! /* successful, copy and store tuple */ ! EvalPlanQualSetTuple(&node->lr_epqstate, erm->rti, ! heap_copytuple(&tuple)); ! ReleaseBuffer(buffer); } /* --- 291,323 ---- /* okay, fetch the tuple */ tuple.t_self = erm->curCtid; ! if (erm->relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE) ! { ! FdwRoutine *fdwroutine; ! ! /* let the FDW do the work */ ! fdwroutine = GetFdwRoutineForRelation(erm->relation, false); ! copyTuple = fdwroutine->FetchForeignRow(node->lr_epqstate.estate, ! erm, &tuple.t_self); ! if (copyTuple == NULL) ! elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); ! } ! else ! { ! Buffer buffer; ! ! if (!heap_fetch(erm->relation, SnapshotAny, &tuple, &buffer, ! false, NULL)) ! elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); ! ! /* successful, copy tuple */ ! copyTuple = heap_copytuple(&tuple); ! ReleaseBuffer(buffer); ! } ! ! /* store tuple */ ! EvalPlanQualSetTuple(&node->lr_epqstate, erm->rti, copyTuple); } /* *************** *** 367,373 **** ExecInitLockRows(LockRows *node, EState *estate, int eflags) continue; /* find ExecRowMark and build ExecAuxRowMark */ ! erm = ExecFindRowMark(estate, rc->rti); aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist); /* --- 410,416 ---- continue; /* find ExecRowMark and build ExecAuxRowMark */ ! erm = ExecFindRowMark(estate, rc->rti, false); aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist); /* *** a/src/backend/executor/nodeModifyTable.c --- b/src/backend/executor/nodeModifyTable.c *************** *** 1257,1263 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) continue; /* find ExecRowMark (same for all subplans) */ ! erm = ExecFindRowMark(estate, rc->rti); /* build ExecAuxRowMark for each subplan */ for (i = 0; i < nplans; i++) --- 1257,1263 ---- continue; /* find ExecRowMark (same for all subplans) */ ! erm = ExecFindRowMark(estate, rc->rti, false); /* build ExecAuxRowMark for each subplan */ for (i = 0; i < nplans; i++) *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *************** *** 20,25 **** --- 20,26 ---- #include "access/htup_details.h" #include "executor/executor.h" #include "executor/nodeAgg.h" + #include "foreign/fdwapi.h" #include "miscadmin.h" #include "nodes/makefuncs.h" #ifdef OPTIMIZER_DEBUG *************** *** 2274,2280 **** select_rowmark_type(RangeTblEntry *rte, LockClauseStrength strength) } else if (rte->relkind == RELKIND_FOREIGN_TABLE) { ! /* For now, we force all foreign tables to use ROW_MARK_COPY */ return ROW_MARK_COPY; } else --- 2275,2288 ---- } else if (rte->relkind == RELKIND_FOREIGN_TABLE) { ! FdwRoutine *fdwroutine; ! ! /* Let the FDW select the rowmark type, if possible */ ! fdwroutine = GetFdwRoutineByRelId(rte->relid); ! if (fdwroutine->GetForeignRowMarkType != NULL) ! return fdwroutine->GetForeignRowMarkType(strength); ! ! /* Otherwise, we force all foreign tables to use ROW_MARK_COPY */ return ROW_MARK_COPY; } else *** a/src/include/executor/executor.h --- b/src/include/executor/executor.h *************** *** 195,201 **** extern void ExecConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern void ExecWithCheckOptions(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); ! extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, Relation relation, Index rti, int lockmode, --- 195,201 ---- TupleTableSlot *slot, EState *estate); extern void ExecWithCheckOptions(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); ! extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok); extern ExecAuxRowMark *ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist); extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate, Relation relation, Index rti, int lockmode, *** a/src/include/foreign/fdwapi.h --- b/src/include/foreign/fdwapi.h *************** *** 13,18 **** --- 13,19 ---- #define FDWAPI_H #include "nodes/execnodes.h" + #include "nodes/plannodes.h" #include "nodes/relation.h" /* To avoid including explain.h here, reference ExplainState thus: */ *************** *** 82,87 **** typedef void (*EndForeignModify_function) (EState *estate, --- 83,98 ---- typedef int (*IsForeignRelUpdatable_function) (Relation rel); + typedef RowMarkType (*GetForeignRowMarkType_function) (LockClauseStrength strength); + + typedef bool (*LockForeignRow_function) (EState *estate, + ExecRowMark *erm, + ItemPointer tupleid); + + typedef HeapTuple (*FetchForeignRow_function) (EState *estate, + ExecRowMark *erm, + ItemPointer tupleid); + typedef void (*ExplainForeignScan_function) (ForeignScanState *node, struct ExplainState *es); *************** *** 141,146 **** typedef struct FdwRoutine --- 152,162 ---- EndForeignModify_function EndForeignModify; IsForeignRelUpdatable_function IsForeignRelUpdatable; + /* Functions for SELECT FOR UPDATE/SHARE row locking */ + GetForeignRowMarkType_function GetForeignRowMarkType; + LockForeignRow_function LockForeignRow; + FetchForeignRow_function FetchForeignRow; + /* Support functions for EXPLAIN */ ExplainForeignScan_function ExplainForeignScan; ExplainForeignModify_function ExplainForeignModify; *** a/src/include/nodes/execnodes.h --- b/src/include/nodes/execnodes.h *************** *** 420,426 **** typedef struct EState * subqueries-in-FROM will have an ExecRowMark with relation == NULL. See * PlanRowMark for details about most of the fields. In addition to fields * directly derived from PlanRowMark, we store curCtid, which is used by the ! * WHERE CURRENT OF code. * * EState->es_rowMarks is a list of these structs. */ --- 420,427 ---- * subqueries-in-FROM will have an ExecRowMark with relation == NULL. See * PlanRowMark for details about most of the fields. In addition to fields * directly derived from PlanRowMark, we store curCtid, which is used by the ! * WHERE CURRENT OF code, and fdw_state, which is used by the FDW if the ! * relation is foreign. * * EState->es_rowMarks is a list of these structs. */ *************** *** 434,439 **** typedef struct ExecRowMark --- 435,441 ---- RowMarkType markType; /* see enum in nodes/plannodes.h */ LockWaitPolicy waitPolicy; /* NOWAIT and SKIP LOCKED */ ItemPointerData curCtid; /* ctid of currently locked tuple, if any */ + void *fdw_state; /* foreign-data wrapper can keep state here */ } ExecRowMark; /*
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers