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 &amp; 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

Reply via email to