From c1a1ca617f344b8e4d6094da50585783508de0c2 Mon Sep 17 00:00:00 2001 From: kommih Date: Wed, 22 Aug 2018 16:45:02 +1000 Subject: [PATCH 3/3] FDW RefetchForeignRow API prototype change With pluggable storage, the tuple usage is minimized and all the extenal API's must deal with TupleTableSlot. --- doc/src/sgml/fdwhandler.sgml | 10 ++++++---- src/backend/executor/execMain.c | 16 ++++++++-------- src/backend/executor/nodeLockRows.c | 20 +++++++------------- src/include/foreign/fdwapi.h | 9 +++++---- 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 4ce88dd77c..12769f3288 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -988,23 +988,25 @@ GetForeignRowMarkType(RangeTblEntry *rte, -HeapTuple +TupleTableSlot * RefetchForeignRow(EState *estate, ExecRowMark *erm, Datum rowid, + TupleTableSlot *slot, bool *updated); - Re-fetch one tuple from the foreign table, after locking it if required. + Re-fetch one tuple slot from the foreign table, after locking it if required. estate is global execution state for the query. erm is the ExecRowMark struct describing the target foreign table and the row lock type (if any) to acquire. rowid identifies the tuple to be fetched. - updated is an output parameter. + slot contains nothing useful upon call, but can be used to + hold the returned tuple. updated is an output parameter. - This function should return a palloc'ed copy of the fetched tuple, + This function should return a slot containing the fetched tuple or NULL if the row lock couldn't be obtained. The row lock type to acquire is defined by erm->markType, which is the value previously returned by GetForeignRowMarkType. diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index faeb960e1d..674569a586 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2705,23 +2705,24 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate) /* fetch requests on foreign tables must be passed to their FDW */ if (erm->relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE) { - elog(ERROR, "frak, need to change fdw API"); -#ifdef FIXME FdwRoutine *fdwroutine; bool updated = false; fdwroutine = GetFdwRoutineForRelation(erm->relation, false); + /* this should have been checked already, but let's be safe */ if (fdwroutine->RefetchForeignRow == NULL) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot lock rows in foreign table \"%s\"", RelationGetRelationName(erm->relation)))); - tuple = fdwroutine->RefetchForeignRow(epqstate->estate, - erm, - datum, - &updated); - if (tuple == NULL) + + slot = fdwroutine->RefetchForeignRow(epqstate->estate, + erm, + datum, + slot, + &updated); + if (slot == NULL) elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck"); /* @@ -2729,7 +2730,6 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate) * assumes that FDWs can track that exactly, which they might * not be able to. So just ignore the flag. */ -#endif } else { diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 668f5fa7a2..e52394a65c 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -128,33 +128,27 @@ lnext: { FdwRoutine *fdwroutine; bool updated = false; - HeapTuple copyTuple; - - elog(ERROR, "frak, tuple based API needs to be rewritten"); fdwroutine = GetFdwRoutineForRelation(erm->relation, false); + /* this should have been checked already, but let's be safe */ if (fdwroutine->RefetchForeignRow == NULL) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot lock rows in foreign table \"%s\"", RelationGetRelationName(erm->relation)))); - copyTuple = fdwroutine->RefetchForeignRow(estate, - erm, - datum, - &updated); - if (copyTuple == NULL) + markSlot = fdwroutine->RefetchForeignRow(estate, + erm, + datum, + markSlot, + &updated); + if (markSlot == NULL) { /* couldn't get the lock, so skip this row */ goto lnext; } - elog(ERROR, "frak: slotify"); - - /* save locked tuple for possible EvalPlanQual testing below */ - //*testTuple = copyTuple; - /* * if FDW says tuple was updated before getting locked, we need to * perform EPQ testing to see if quals are still satisfied diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h index c14eb546c6..508b0eece8 100644 --- a/src/include/foreign/fdwapi.h +++ b/src/include/foreign/fdwapi.h @@ -121,10 +121,11 @@ typedef void (*EndDirectModify_function) (ForeignScanState *node); typedef RowMarkType (*GetForeignRowMarkType_function) (RangeTblEntry *rte, LockClauseStrength strength); -typedef HeapTuple (*RefetchForeignRow_function) (EState *estate, - ExecRowMark *erm, - Datum rowid, - bool *updated); +typedef TupleTableSlot *(*RefetchForeignRow_function) (EState *estate, + ExecRowMark *erm, + Datum rowid, + TupleTableSlot *slot, + bool *updated); typedef void (*ExplainForeignScan_function) (ForeignScanState *node, struct ExplainState *es); -- 2.18.0.windows.1