Re: SQL:2011 Application Time Update & Delete - Mailing list pgsql-hackers

Peter Eisentraut <peter@eisentraut.org> writes:
> I have committed the patches 0001 through 0003.

Coverity is complaining that rsi.isDone may be used uninitialized in
ExecForPortionOfLeftovers.  It's correct: that function is not obeying
the function call protocol, and it's only accidental that it's not
failing.  In ValuePerCall mode the caller is supposed to initialize
isDone (and isnull too) before each call.  The canonical reference
for this is execSRF.c, and it does that.  So I think we need something
like the attached.

I notice that execSRF.c also runs pgstat_init_function_usage and
pgstat_end_function_usage around each call.  That's not too important
right now, but I wonder whether we should add it while we're looking
at this.  It would perhaps be important once we support user-defined
withoutPortionProcs.

            regards, tom lane

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ef2a6bc6e9d..b852b96839d 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1514,6 +1514,7 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
     rsi.expectedDesc = NULL;
     rsi.allowedModes = (int) (SFRM_ValuePerCall);
     rsi.returnMode = SFRM_ValuePerCall;
+    /* isDone is filled below */
     rsi.setResult = NULL;
     rsi.setDesc = NULL;

@@ -1537,14 +1538,22 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
      */
     while (true)
     {
-        Datum        leftover = FunctionCallInvoke(fcinfo);
+        Datum        leftover;
+
+        /* Call the function one time */
+        fcinfo->isnull = false;
+        rsi.isDone = ExprSingleResult;
+        leftover = FunctionCallInvoke(fcinfo);
+
+        if (rsi.returnMode != SFRM_ValuePerCall)
+            elog(ERROR, "without_portion function violated function call protocol");

         /* Are we done? */
         if (rsi.isDone == ExprEndResult)
             break;

         if (fcinfo->isnull)
-            elog(ERROR, "Got a null from without_portion function");
+            elog(ERROR, "got a null from without_portion function");

         /*
          * Does the new Datum violate domain checks? Row-level CHECK