On 2015/04/15 2:27, Jim Nasby wrote:
On 4/14/15 1:05 AM, Kyotaro HORIGUCHI wrote:
As an example, the following operations cause an "unexpected"
result.
Those results are indeed surprising, but since we allow it in a direct
connection I don't see why we wouldn't allow it in the Postgres FDW...
As for the FDW not knowing about keys, why would it need to? If you try
to do something illegal it's the remote side that should throw the
error, not the FDW.
Of course, if you try to do a locking operation on an FDW that doesn't
support it, the FDW should throw an error... but that's different.
Ah, you are right. FOR NO KEY UPDATE and FOR KEY SHARE would be useful
in the Postgres FDW if we assume the user performs those properly based
on information about keys for a remote table.
Sorry, my explanation was not correct, but I want to make it clear that
the proposed patch also allows the FDW to change the behavior of FOR NO
KEY UPDATE and/or FOR KEY SHARE row locking so as to match the local
semantics exactly.
BTW, I revised docs a bit. Attached is 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,232 ----
</para>
<para>
+ If the FDW changes handling <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 the command or not, it's recommended to use
+ <function>ExecFindRowMark</> with the missing_ok argument set to true,
+ which returns an <structname>ExecRowMark</> struct if the foreign table
+ is referenced in the command and otherwise returns a NULL. Information
+ about the row locking is accessible through the
+ <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 row
+ locking.)
+ </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);
--- 614,701 ----
</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>
--- 1109,1118 ----
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
