Improve UPDATE/DELETE WHERE CURRENT OF so that they can be used from plpgsql

with a plpgsql-defined cursor.  The underlying mechanism for this is that the
main SQL engine will now take "WHERE CURRENT OF $n" where $n is a refcursor
parameter.  Not sure if we should document that fact or consider it an
implementation detail.  Per discussion with Pavel Stehule.

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 671efafc18c7e5b0c5c4f55709fa2572f20e80f0..45460c1f17d3649076c8cbfde7fc0db36e40f565 100644 (file)
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -2614,6 +2614,31 @@ MOVE <optional> <replaceable>direction</replaceable> { FROM | IN } </optional> <
 MOVE curs1;
 MOVE LAST FROM curs3;
 MOVE RELATIVE -2 FROM curs4;
+</programlisting>
+       </para>
+     </sect3>
+
+    <sect3>
+     <title><literal>UPDATE/DELETE WHERE CURRENT OF</></title>
+
+<synopsis>
+UPDATE <replaceable>table</replaceable> SET ... WHERE CURRENT OF <replaceable>cursor</replaceable>;
+DELETE FROM <replaceable>table</replaceable> WHERE CURRENT OF <replaceable>cursor</replaceable>;
+</synopsis>
+
+       <para>
+        When a cursor is positioned on a table row, that row can be updated
+        or deleted using the cursor to identify the row.  Note that this
+        only works for simple (non-join, non-grouping) cursor queries.
+        For additional information see the
+        <xref linkend="sql-declare" endterm="sql-declare-title">
+        reference page.
+       </para>
+
+       <para>
+        An example:
+<programlisting>
+UPDATE foo SET dataval = myval WHERE CURRENT OF curs1;
 </programlisting>
        </para>
      </sect3>

diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.c
index d2c79e7da9c8619bc3c0fc1ee739b43a3888b4d2..e10b2af128f04a45db140c2b7eabe2d627c11c8f 100644 (file)
--- a/src/backend/executor/execCurrent.c
+++ b/src/backend/executor/execCurrent.c
@@ -12,20 +12,23 @@
  */
 #include "postgres.h"
 
+#include "catalog/pg_type.h"
 #include "executor/executor.h"
+#include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/portal.h"
 
 
+static char *fetch_param_value(ExprContext *econtext, int paramId);
 static ScanState *search_plan_tree(PlanState *node, Oid table_oid);
 
 
 /*
  * execCurrentOf
  *
- * Given the name of a cursor and the OID of a table, determine which row
- * of the table is currently being scanned by the cursor, and return its
- * TID into *current_tid.
+ * Given a CURRENT OF expression and the OID of a table, determine which row
+ * of the table is currently being scanned by the cursor named by CURRENT OF,
+ * and return the row's TID into *current_tid.
  *
  * Returns TRUE if a row was identified.  Returns FALSE if the cursor is valid
  * for the table but is not currently scanning a row of the table (this is a
@@ -33,14 +36,25 @@ static ScanState *search_plan_tree(PlanState *node, Oid table_oid);
  * valid updatable scan of the specified table.
  */
 bool
-execCurrentOf(char *cursor_name, Oid table_oid,
+execCurrentOf(CurrentOfExpr *cexpr,
+                         ExprContext *econtext,
+                         Oid table_oid,
                          ItemPointer current_tid)
 {
+       char       *cursor_name;
        char       *table_name;
        Portal          portal;
        QueryDesc *queryDesc;
        ScanState  *scanstate;
-       HeapTuple tup;
+       bool    lisnull;
+       Oid             tuple_tableoid;
+       ItemPointer tuple_tid;
+
+       /* Get the cursor name --- may have to look up a parameter reference */
+       if (cexpr->cursor_name)
+               cursor_name = cexpr->cursor_name;
+       else
+               cursor_name = fetch_param_value(econtext, cexpr->cursor_param);
 
        /* Fetch table name for possible use in error messages */
        table_name = get_rel_name(table_oid);
@@ -100,16 +114,54 @@ execCurrentOf(char *cursor_name, Oid table_oid,
        if (TupIsNull(scanstate->ss_ScanTupleSlot))
                return false;
 
-       tup = scanstate->ss_ScanTupleSlot->tts_tuple;
-       if (tup == NULL)
-               elog(ERROR, "CURRENT OF applied to non-materialized tuple");
-       Assert(tup->t_tableOid == table_oid);
+       /* Use slot_getattr to catch any possible mistakes */
+       tuple_tableoid = DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
+                                                                                                  TableOidAttributeNumber,
+                                                                                                  &lisnull));
+       Assert(!lisnull);
+       tuple_tid = (ItemPointer)
+               DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
+                                                                        SelfItemPointerAttributeNumber,
+                                                                        &lisnull));
+       Assert(!lisnull);
+
+       Assert(tuple_tableoid == table_oid);
 
-       *current_tid = tup->t_self;
+       *current_tid = *tuple_tid;
 
        return true;
 }
 
+/*
+ * fetch_param_value
+ *
+ * Fetch the string value of a param, verifying it is of type REFCURSOR.
+ */
+static char *
+fetch_param_value(ExprContext *econtext, int paramId)
+{
+       ParamListInfo paramInfo = econtext->ecxt_param_list_info;
+
+       if (paramInfo &&
+               paramId > 0 && paramId <= paramInfo->numParams)
+       {
+               ParamExternData *prm = &paramInfo->params[paramId - 1];
+
+               if (OidIsValid(prm->ptype) && !prm->isnull)
+               {
+                       Assert(prm->ptype == REFCURSOROID);
+                       /* We know that refcursor uses text's I/O routines */
+                       return DatumGetCString(DirectFunctionCall1(textout,
+                                                                                                          prm->value));
+               }
+       }
+
+       ereport(ERROR,
+                       (errcode(ERRCODE_UNDEFINED_OBJECT),
+                        errmsg("no value found for parameter %d", paramId)));
+       return NULL;
+}
+
 /*
  * search_plan_tree
  *

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index e70663f0b8d4602308c19a22a1fbe479029d91fb..4f6663e0f22ae467ca7fcc5b4576b674eb34d55b 100644 (file)
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -3632,8 +3632,10 @@ ExecEvalCurrentOfExpr(ExprState *exprstate, ExprContext *econtext,
                                          bool *isNull, ExprDoneCond *isDone)
 {
        CurrentOfExpr *cexpr = (CurrentOfExpr *) exprstate->expr;
-       bool result;
-       HeapTuple tup;
+       bool    result;
+       bool    lisnull;
+       Oid             tableoid;
+       ItemPointer tuple_tid;
        ItemPointerData cursor_tid;
 
        if (isDone)
@@ -3643,12 +3645,19 @@ ExecEvalCurrentOfExpr(ExprState *exprstate, ExprContext *econtext,
        Assert(cexpr->cvarno != INNER);
        Assert(cexpr->cvarno != OUTER);
        Assert(!TupIsNull(econtext->ecxt_scantuple));
-       tup = econtext->ecxt_scantuple->tts_tuple;
-       if (tup == NULL)
-               elog(ERROR, "CURRENT OF applied to non-materialized tuple");
-
-       if (execCurrentOf(cexpr->cursor_name, tup->t_tableOid, &cursor_tid))
-               result = ItemPointerEquals(&cursor_tid, &(tup->t_self));
+       /* Use slot_getattr to catch any possible mistakes */
+       tableoid = DatumGetObjectId(slot_getattr(econtext->ecxt_scantuple,
+                                                                                        TableOidAttributeNumber,
+                                                                                        &lisnull));
+       Assert(!lisnull);
+       tuple_tid = (ItemPointer)
+               DatumGetPointer(slot_getattr(econtext->ecxt_scantuple,
+                                                                        SelfItemPointerAttributeNumber,
+                                                                        &lisnull));
+       Assert(!lisnull);
+
+       if (execCurrentOf(cexpr, econtext, tableoid, &cursor_tid))
+               result = ItemPointerEquals(&cursor_tid, tuple_tid);
        else
                result = false;
 

diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index d9c903d543d34f8f3bdbe97844379989a0c7de1a..bff8a9b21d577e7729383114156329720ec47e15 100644 (file)
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -153,7 +153,7 @@ TidListCreate(TidScanState *tidstate)
                        CurrentOfExpr *cexpr = (CurrentOfExpr *) expr;
                        ItemPointerData cursor_tid;
 
-                       if (execCurrentOf(cexpr->cursor_name,
+                       if (execCurrentOf(cexpr, econtext,
                                                  RelationGetRelid(tidstate->ss.ss_currentRelation),
                                                          &cursor_tid))
                        {

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index c39b516c7ab4741aa8b885e01355d8df3503e9eb..2a4d1947d003aa3b988aae8cc0d76da8340d88a7 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1309,6 +1309,7 @@ _copyCurrentOfExpr(CurrentOfExpr *from)
 
        COPY_SCALAR_FIELD(cvarno);
        COPY_STRING_FIELD(cursor_name);
+       COPY_SCALAR_FIELD(cursor_param);
 
        return newnode;
 }

diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 1cedee670aa62ba965928ff33fc35071983d3263..d35bbd12d22cf79f6557e164797727edd320a26e 100644 (file)
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -603,6 +603,7 @@ _equalCurrentOfExpr(CurrentOfExpr *a, CurrentOfExpr *b)
 {
        COMPARE_SCALAR_FIELD(cvarno);
        COMPARE_STRING_FIELD(cursor_name);
+       COMPARE_SCALAR_FIELD(cursor_param);
 
        return true;
 }

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 2edb53e2f3713145808650b7c728320ce3b46cf6..3e892eee10b40dd0a0f3663297990e5061e01e88 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1065,6 +1065,7 @@ _outCurrentOfExpr(StringInfo str, CurrentOfExpr *node)
 
        WRITE_UINT_FIELD(cvarno);
        WRITE_STRING_FIELD(cursor_name);
+       WRITE_INT_FIELD(cursor_param);
 }
 
 static void

diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 141a325b6e5b41bfb58d91e47640f94d09ef02d9..49f0c470f002f3ab3b0f97609506a483d3e45bd4 100644 (file)
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -883,6 +883,7 @@ _readCurrentOfExpr(void)
 
        READ_UINT_FIELD(cvarno);
        READ_STRING_FIELD(cursor_name);
+       READ_INT_FIELD(cursor_param);
 
        READ_DONE();
 }

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c0b498f0e519ab0faf804d83924c2c9e6f60ddbd..dbe2dd3b98afa51209331e7055d7c7416465cd09 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6568,7 +6568,17 @@ where_or_current_clause:
                        | WHERE CURRENT_P OF name
                                {
                                        CurrentOfExpr *n = makeNode(CurrentOfExpr);
+                                       /* cvarno is filled in by parse analysis */
                                        n->cursor_name = $4;
+                                       n->cursor_param = 0;
+                                       $$ = (Node *) n;
+                               }
+                       | WHERE CURRENT_P OF PARAM
+                               {
+                                       CurrentOfExpr *n = makeNode(CurrentOfExpr);
+                                       /* cvarno is filled in by parse analysis */
+                                       n->cursor_name = NULL;
+                                       n->cursor_param = $4;
                                        $$ = (Node *) n;
                                }
                        | /*EMPTY*/                                                             { $$ = NULL; }

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index b26719f330122524e97c7dd19cc3640195b88758..a7efe605768c2f129958dc2c2f7cf2f85ae26aa5 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -59,10 +59,10 @@ static Node *transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m);
 static Node *transformXmlExpr(ParseState *pstate, XmlExpr *x);
 static Node *transformXmlSerialize(ParseState *pstate, XmlSerialize *xs);
 static Node *transformBooleanTest(ParseState *pstate, BooleanTest *b);
+static Node *transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr);
 static Node *transformColumnRef(ParseState *pstate, ColumnRef *cref);
 static Node *transformWholeRowRef(ParseState *pstate, char *schemaname,
                                         char *relname, int location);
-static Node *transformBooleanTest(ParseState *pstate, BooleanTest *b);
 static Node *transformIndirection(ParseState *pstate, Node *basenode,
                                         List *indirection);
 static Node *typecast_expression(ParseState *pstate, Node *expr,
@@ -244,19 +244,8 @@ transformExpr(ParseState *pstate, Node *expr)
                        break;
 
                case T_CurrentOfExpr:
-                       {
-                               CurrentOfExpr *c = (CurrentOfExpr *) expr;
-                               int             sublevels_up;
-
-                               /* CURRENT OF can only appear at top level of UPDATE/DELETE */
-                               Assert(pstate->p_target_rangetblentry != NULL);
-                               c->cvarno = RTERangeTablePosn(pstate,
-                                                                                         pstate->p_target_rangetblentry,
-                                                                                         &sublevels_up);
-                               Assert(sublevels_up == 0);
-                               result = expr;
-                               break;
-                       }
+                       result = transformCurrentOfExpr(pstate, (CurrentOfExpr *) expr);
+                       break;
 
                        /*********************************************
                         * Quietly accept node types that may be presented when we are
@@ -549,57 +538,69 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
        return node;
 }
 
-static Node *
-transformParamRef(ParseState *pstate, ParamRef *pref)
+/*
+ * Locate the parameter type info for the given parameter number, and
+ * return a pointer to it.
+ */
+static Oid *
+find_param_type(ParseState *pstate, int paramno)
 {
-       int                     paramno = pref->number;
-       ParseState *toppstate;
-       Param      *param;
+       Oid        *result;
 
        /*
         * Find topmost ParseState, which is where paramtype info lives.
         */
-       toppstate = pstate;
-       while (toppstate->parentParseState != NULL)
-               toppstate = toppstate->parentParseState;
+       while (pstate->parentParseState != NULL)
+               pstate = pstate->parentParseState;
 
        /* Check parameter number is in range */
        if (paramno <= 0)                       /* probably can't happen? */
                ereport(ERROR,
                                (errcode(ERRCODE_UNDEFINED_PARAMETER),
                                 errmsg("there is no parameter $%d", paramno)));
-       if (paramno > toppstate->p_numparams)
+       if (paramno > pstate->p_numparams)
        {
-               if (!toppstate->p_variableparams)
+               if (!pstate->p_variableparams)
                        ereport(ERROR,
                                        (errcode(ERRCODE_UNDEFINED_PARAMETER),
                                         errmsg("there is no parameter $%d",
                                                        paramno)));
                /* Okay to enlarge param array */
-               if (toppstate->p_paramtypes)
-                       toppstate->p_paramtypes =
-                               (Oid *) repalloc(toppstate->p_paramtypes,
-                                                                paramno * sizeof(Oid));
+               if (pstate->p_paramtypes)
+                       pstate->p_paramtypes = (Oid *) repalloc(pstate->p_paramtypes,
+                                                                                                       paramno * sizeof(Oid));
                else
-                       toppstate->p_paramtypes =
-                               (Oid *) palloc(paramno * sizeof(Oid));
+                       pstate->p_paramtypes = (Oid *) palloc(paramno * sizeof(Oid));
                /* Zero out the previously-unreferenced slots */
-               MemSet(toppstate->p_paramtypes + toppstate->p_numparams,
+               MemSet(pstate->p_paramtypes + pstate->p_numparams,
                           0,
-                          (paramno - toppstate->p_numparams) * sizeof(Oid));
-               toppstate->p_numparams = paramno;
+                          (paramno - pstate->p_numparams) * sizeof(Oid));
+               pstate->p_numparams = paramno;
        }
-       if (toppstate->p_variableparams)
+
+       result = &pstate->p_paramtypes[paramno - 1];
+
+       if (pstate->p_variableparams)
        {
                /* If not seen before, initialize to UNKNOWN type */
-               if (toppstate->p_paramtypes[paramno - 1] == InvalidOid)
-                       toppstate->p_paramtypes[paramno - 1] = UNKNOWNOID;
+               if (*result == InvalidOid)
+                       *result = UNKNOWNOID;
        }
 
+       return result;
+}
+
+static Node *
+transformParamRef(ParseState *pstate, ParamRef *pref)
+{
+       int                     paramno = pref->number;
+       Oid                *pptype = find_param_type(pstate, paramno);
+       Param      *param;
+
        param = makeNode(Param);
        param->paramkind = PARAM_EXTERN;
        param->paramid = paramno;
-       param->paramtype = toppstate->p_paramtypes[paramno - 1];
+       param->paramtype = *pptype;
        param->paramtypmod = -1;
 
        return (Node *) param;
@@ -1596,6 +1597,43 @@ transformBooleanTest(ParseState *pstate, BooleanTest *b)
        return (Node *) b;
 }
 
+static Node *
+transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr)
+{
+       int             sublevels_up;
+
+       /* CURRENT OF can only appear at top level of UPDATE/DELETE */
+       Assert(pstate->p_target_rangetblentry != NULL);
+       cexpr->cvarno = RTERangeTablePosn(pstate,
+                                                                         pstate->p_target_rangetblentry,
+                                                                         &sublevels_up);
+       Assert(sublevels_up == 0);
+
+       /* If a parameter is used, it must be of type REFCURSOR */
+       if (cexpr->cursor_name == NULL)
+       {
+               Oid                *pptype = find_param_type(pstate, cexpr->cursor_param);
+
+               if (pstate->p_variableparams && *pptype == UNKNOWNOID)
+               {
+                       /* resolve unknown param type as REFCURSOR */
+                       *pptype = REFCURSOROID;
+               }
+               else if (*pptype != REFCURSOROID)
+               {
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_AMBIGUOUS_PARAMETER),
+                                        errmsg("inconsistent types deduced for parameter $%d",
+                                                       cexpr->cursor_param),
+                                        errdetail("%s versus %s",
+                                                          format_type_be(*pptype),
+                                                          format_type_be(REFCURSOROID))));
+               }
+       }
+
+       return (Node *) cexpr;
+}
+
 /*
  * Construct a whole-row reference to represent the notation "relation.*".
  *

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index d22fe4733d01d4a4fb4573841994835252f9981e..3988f45b67d20c5a1e63d9ebab821b099e7a985b 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -4136,8 +4136,16 @@ get_rule_expr(Node *node, deparse_context *context,
                        break;
 
                case T_CurrentOfExpr:
-                       appendStringInfo(buf, "CURRENT OF %s",
-                                       quote_identifier(((CurrentOfExpr *) node)->cursor_name));
+                       {
+                               CurrentOfExpr *cexpr = (CurrentOfExpr *) node;
+
+                               if (cexpr->cursor_name)
+                                       appendStringInfo(buf, "CURRENT OF %s",
+                                                                        quote_identifier(cexpr->cursor_name));
+                               else
+                                       appendStringInfo(buf, "CURRENT OF $%d",
+                                                                        cexpr->cursor_param);
+                       }
                        break;
 
                case T_List:

diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index c7ecc4c55bdb5601adde87c54ce88d12281c5a7b..351787ebf6d58896876bc7fefcc16944fa613cae 100644 (file)
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -73,7 +73,9 @@ extern bool ExecMayReturnRawTuples(PlanState *node);
 /*
  * prototypes from functions in execCurrent.c
  */
-extern bool execCurrentOf(char *cursor_name, Oid table_oid,
+extern bool execCurrentOf(CurrentOfExpr *cexpr,
+                                                 ExprContext *econtext,
+                                                 Oid table_oid,
                                                  ItemPointer current_tid);
 
 /*

diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 24a840c9ade50dca7f0d0a2b9e73ef9e50655666..02880657eb77a0223d516201eefd10800d30b399 100644 (file)
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -922,12 +922,17 @@ typedef struct SetToDefault
  * of the target relation being constrained; this aids placing the expression
  * correctly during planning.  We can assume however that its "levelsup" is
  * always zero, due to the syntactic constraints on where it can appear.
+ *
+ * The referenced cursor can be represented either as a hardwired string
+ * or as a reference to a run-time parameter of type REFCURSOR.  The latter
+ * case is for the convenience of plpgsql.
  */
 typedef struct CurrentOfExpr
 {
        Expr            xpr;
        Index           cvarno;                 /* RT index of target relation */
-       char       *cursor_name;        /* name of referenced cursor */
+       char       *cursor_name;        /* name of referenced cursor, or NULL */
+       int                     cursor_param;   /* refcursor parameter number, or 0 */
 } CurrentOfExpr;
 
 /*--------------------