Re: pg_stop_backup() v2 incorrectly marked as proretset

[Michael Paquier]

On Wed, Mar 02, 2022 at 05:22:35PM +0900, Kyotaro Horiguchi wrote:
> But the patch forgets to remove an useless variable.

Indeed.  I forgot to look at stderr.

>>      /* Initialise attributes information in the tuple descriptor */
>>      tupdesc = CreateTemplateTupleDesc(PG_STOP_BACKUP_V2_COLS);
>>      TupleDescInitEntry(tupdesc, (AttrNumber) 1, "lsn",
>>                                         PG_LSNOID, -1, 0);
> 
> I think we can use get_call_resuilt_type here.

Yes, I don't mind doing so here.
--
Michael

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index bf88858171..d8e8715ed1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6275,9 +6275,9 @@
   proname => 'pg_stop_backup', provolatile => 'v', proparallel => 'r',
   prorettype => 'pg_lsn', proargtypes => '', prosrc => 'pg_stop_backup' },
 { oid => '2739', descr => 'finish taking an online backup',
-  proname => 'pg_stop_backup', prorows => '1', proretset => 't',
-  provolatile => 'v', proparallel => 'r', prorettype => 'record',
-  proargtypes => 'bool bool', proallargtypes => '{bool,bool,pg_lsn,text,text}',
+  proname => 'pg_stop_backup', provolatile => 'v', proparallel => 'r',
+  prorettype => 'record', proargtypes => 'bool bool',
+  proallargtypes => '{bool,bool,pg_lsn,text,text}',
   proargmodes => '{i,i,o,o,o}',
   proargnames => '{exclusive,wait_for_archive,lsn,labelfile,spcmapfile}',
   prosrc => 'pg_stop_backup_v2' },
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 12e2bf4135..9f7a282ed2 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -165,43 +165,20 @@ pg_stop_backup(PG_FUNCTION_ARGS)
 Datum
 pg_stop_backup_v2(PG_FUNCTION_ARGS)
 {
-	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+#define PG_STOP_BACKUP_V2_COLS 3
 	TupleDesc	tupdesc;
-	Tuplestorestate *tupstore;
-	MemoryContext per_query_ctx;
-	MemoryContext oldcontext;
-	Datum		values[3];
-	bool		nulls[3];
+	Datum		values[PG_STOP_BACKUP_V2_COLS];
+	bool		nulls[PG_STOP_BACKUP_V2_COLS];
 
 	bool		exclusive = PG_GETARG_BOOL(0);
 	bool		waitforarchive = PG_GETARG_BOOL(1);
 	XLogRecPtr	stoppoint;
 	SessionBackupState status = get_backup_status();
 
-	/* check to see if caller supports us returning a tuplestore */
-	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("set-valued function called in context that cannot accept a set")));
-	if (!(rsinfo->allowedModes & SFRM_Materialize))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("materialize mode required, but it is not allowed in this context")));
-
-	/* Build a tuple descriptor for our result type */
+	/* Initialise attributes information in the tuple descriptor */
 	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
-	per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
-	oldcontext = MemoryContextSwitchTo(per_query_ctx);
-
-	tupstore = tuplestore_begin_heap(true, false, work_mem);
-	rsinfo->returnMode = SFRM_Materialize;
-	rsinfo->setResult = tupstore;
-	rsinfo->setDesc = tupdesc;
-
-	MemoryContextSwitchTo(oldcontext);
-
 	MemSet(values, 0, sizeof(values));
 	MemSet(nulls, 0, sizeof(nulls));
 
@@ -251,9 +228,8 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
 	/* Stoppoint is included on both exclusive and nonexclusive backups */
 	values[0] = LSNGetDatum(stoppoint);
 
-	tuplestore_putvalues(tupstore, tupdesc, values, nulls);
-
-	return (Datum) 0;
+	/* Returns the record as Datum */
+	PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls)));
 }
 
 /*
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 758ab6e25a..81bac6f581 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -384,7 +384,7 @@ CREATE OR REPLACE FUNCTION
 CREATE OR REPLACE FUNCTION pg_stop_backup (
         exclusive boolean, wait_for_archive boolean DEFAULT true,
         OUT lsn pg_lsn, OUT labelfile text, OUT spcmapfile text)
-  RETURNS SETOF record STRICT VOLATILE LANGUAGE internal as 'pg_stop_backup_v2'
+  RETURNS record STRICT VOLATILE LANGUAGE internal as 'pg_stop_backup_v2'
   PARALLEL RESTRICTED;
 
 CREATE OR REPLACE FUNCTION

signature.asc
Description: PGP signature