pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Paul A Jungwirth <pj@illuminatedcomputing.com>
Cc: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: SQL:2011 Application Time Update & Delete
Date: Sun, 19 Apr 2026 14:10:02 -0400
Message-ID: <4126231.1776622202@sss.pgh.pa.us> (raw)
In-Reply-To: <e2fc32a4-319b-4176-8bd8-1d974e31376f@eisentraut.org>
References: <ec498c3d-5f2b-48ec-b989-5561c8aa2024@illuminatedcomputing.com>
	<85ac7f0e-d95f-4377-ade0-8941fd328012@eisentraut.org>
	<CA+renyUazgR-hB_6RY60n23L0y-n_h9G1AappZmPENO0k5pL1g@mail.gmail.com>
	<7d63ddfa-c735-4dfe-8c7a-4f1e2a621058@eisentraut.org>
	<CA+renyVXg5pV84wQnGQuK8-=qoKw3BiBgQzesxM_LkcxxWmYjA@mail.gmail.com>
	<e59d218a-ae49-490f-ae6f-86c12f35d3c3@eisentraut.org>
	<CA+renyWKOj5=rMmQmJcbybu-Vdomxdp=eJ93kp76AgmQKYdfiQ@mail.gmail.com>
	<4606deaa-7d65-4f22-8a78-356c3180be9d@eisentraut.org>
	<CA+renyUhuXB2nTVCMREXew9E4DZOnFxQNjME5bcw91+k72Bosg@mail.gmail.com>
	<CA+renyWUCSyTMn3s03kviEN-oaVrJP-QkDQCLNfaY=MHV5QEiQ@mail.gmail.com>
	<CA+renyV4tWU2d=n9_v=XNPHbZfNqqLokzd-Xt78M-zLd+46ubA@mail.gmail.com>
	<53f1c094-3c29-4ef6-a9bd-dc2e7894ceb0@eisentraut.org>
	<CA+renyUSgqXpjj+vV7w+wirPB49VQFrmPjVT_s04JmZSOPNNsQ@mail.gmail.com>
	<b2ecbf9f-797f-433d-953f-3c848cde5a1c@eisentraut.org>
	<CA+renyX-eV+2hFUaZg3BSREqLE7dh+LoWm7ZqhFAiGsirjjtRQ@mail.gmail.com>
	<e2fc32a4-319b-4176-8bd8-1d974e31376f! @eisentraut.org>

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

Attachments:

  [text/x-diff] v1-make-ExecForPortionOfLeftovers-obey-protocol.patch (1.2K, ../4126231.1776622202@sss.pgh.pa.us/2-v1-make-ExecForPortionOfLeftovers-obey-protocol.patch)
  download | inline diff:
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

view thread (68+ messages)  latest in thread

Message-ID: <4126231.1776622202@sss.pgh.pa.us>
Permalink:  ../4126231.1776622202@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/4126231.1776622202@sss.pgh.pa.us

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, peter@eisentraut.org, pj@illuminatedcomputing.com, li.evan.chao@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: SQL:2011 Application Time Update & Delete
  In-Reply-To: <4126231.1776622202@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox