Clean up and simplify code in a couple of set-returning functions
authorMichael Paquier <michael@paquier.xyz>
Thu, 24 Feb 2022 07:11:34 +0000 (16:11 +0900)
committerMichael Paquier <michael@paquier.xyz>
Thu, 24 Feb 2022 07:11:34 +0000 (16:11 +0900)
The following set-returning functions have their logic simplified, to be
more consistent with other in-core areas:
- pg_prepared_statement()'s tuple descriptor is now created with
get_call_result_type() instead of being created from scratch, saving
from some duplication with pg_proc.dat.
- show_all_file_settings(), similarly, now uses get_call_result_type()
to build its tuple descriptor instead of creating it from scratch.
- pg_options_to_table() made use of a static routine called only once.
This commit removes this internal routine to make the function easier to
follow.
- pg_config() was using a unique logic style, doing checks on the tuple
descriptor passed down in expectedDesc, but it has no need to do so.
This switches the function to use a tuplestore with a tuple descriptor
retrieved from get_call_result_type(), instead.

This simplifies an upcoming patch aimed at refactoring the way
tuplestores are created and checked in set-returning functions, this
change making sense as its own independent cleanup by shaving some
code.

Author: Melanie Plageman, Michael Paquier
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com

src/backend/commands/prepare.c
src/backend/foreign/foreign.c
src/backend/utils/misc/guc.c
src/backend/utils/misc/pg_config.c
src/backend/utils/mmgr/portalmem.c

index e0c985ef8b08b5ee4211e89f5830ff35bb29d2f6..dce30aed6c1c19aaaf06b177996d2ce361b38681 100644 (file)
@@ -22,6 +22,7 @@
 #include "catalog/pg_type.h"
 #include "commands/createas.h"
 #include "commands/prepare.h"
+#include "funcapi.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/analyze.h"
@@ -716,30 +717,13 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("materialize mode required, but it is not allowed in this context")));
 
+       if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+               elog(ERROR, "return type must be a row type");
+
        /* need to build tuplestore in query context */
        per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
        oldcontext = MemoryContextSwitchTo(per_query_ctx);
 
-       /*
-        * build tupdesc for result tuples. This must match the definition of the
-        * pg_prepared_statements view in system_views.sql
-        */
-       tupdesc = CreateTemplateTupleDesc(7);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 2, "statement",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 3, "prepare_time",
-                                          TIMESTAMPTZOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 4, "parameter_types",
-                                          REGTYPEARRAYOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 5, "from_sql",
-                                          BOOLOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 6, "generic_plans",
-                                          INT8OID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 7, "custom_plans",
-                                          INT8OID, -1, 0);
-
        /*
         * We put all the tuples into a tuplestore in one scan of the hashtable.
         * This avoids any issue of the hashtable possibly changing between calls.
@@ -747,6 +731,9 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
        tupstore =
                tuplestore_begin_heap(rsinfo->allowedModes & SFRM_Materialize_Random,
                                                          false, work_mem);
+       rsinfo->returnMode = SFRM_Materialize;
+       rsinfo->setResult = tupstore;
+       rsinfo->setDesc = tupdesc;
 
        /* generate junk in short-term context */
        MemoryContextSwitchTo(oldcontext);
@@ -778,10 +765,6 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
                }
        }
 
-       rsinfo->returnMode = SFRM_Materialize;
-       rsinfo->setResult = tupstore;
-       rsinfo->setDesc = tupdesc;
-
        return (Datum) 0;
 }
 
index d910bc2fbea86763456e2db992d74c8d79a2e7fb..c3406c3b9d9ba5316b40d922c8cea4a884167686 100644 (file)
@@ -499,17 +499,19 @@ IsImportableForeignTable(const char *tablename,
 
 
 /*
- * deflist_to_tuplestore - Helper function to convert DefElem list to
- * tuplestore usable in SRF.
+ * pg_options_to_table - Convert options array to name/value table
+ *
+ * This is useful to provide details for information_schema and pg_dump.
  */
-static void
-deflist_to_tuplestore(ReturnSetInfo *rsinfo, List *options)
+Datum
+pg_options_to_table(PG_FUNCTION_ARGS)
 {
+       Datum           array = PG_GETARG_DATUM(0);
        ListCell   *cell;
+       List       *options;
+       ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
        TupleDesc       tupdesc;
        Tuplestorestate *tupstore;
-       Datum           values[2];
-       bool            nulls[2];
        MemoryContext per_query_ctx;
        MemoryContext oldcontext;
 
@@ -524,6 +526,9 @@ deflist_to_tuplestore(ReturnSetInfo *rsinfo, List *options)
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("materialize mode required, but it is not allowed in this context")));
 
+       options = untransformRelOptions(array);
+       rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
        per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
        oldcontext = MemoryContextSwitchTo(per_query_ctx);
 
@@ -536,9 +541,13 @@ deflist_to_tuplestore(ReturnSetInfo *rsinfo, List *options)
        rsinfo->setResult = tupstore;
        rsinfo->setDesc = tupdesc;
 
+       MemoryContextSwitchTo(oldcontext);
+
        foreach(cell, options)
        {
                DefElem    *def = lfirst(cell);
+               Datum           values[2];
+               bool            nulls[2];
 
                values[0] = CStringGetTextDatum(def->defname);
                nulls[0] = false;
@@ -555,22 +564,6 @@ deflist_to_tuplestore(ReturnSetInfo *rsinfo, List *options)
                tuplestore_putvalues(tupstore, tupdesc, values, nulls);
        }
 
-       MemoryContextSwitchTo(oldcontext);
-}
-
-
-/*
- * Convert options array to name/value table.  Useful for information
- * schema and pg_dump.
- */
-Datum
-pg_options_to_table(PG_FUNCTION_ARGS)
-{
-       Datum           array = PG_GETARG_DATUM(0);
-
-       deflist_to_tuplestore((ReturnSetInfo *) fcinfo->resultinfo,
-                                                 untransformRelOptions(array));
-
        return (Datum) 0;
 }
 
index bf7ec0d4666022ccab930f1172dd0eba16bc1adf..1e3650184b1336659e9027f600dea1b80e88ffda 100644 (file)
@@ -10174,6 +10174,9 @@ show_all_file_settings(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("materialize mode required, but it is not allowed in this context")));
 
+       if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+               elog(ERROR, "return type must be a row type");
+
        /* Scan the config files using current context as workspace */
        conf = ProcessConfigFileInternal(PGC_SIGHUP, false, DEBUG3);
 
@@ -10181,23 +10184,6 @@ show_all_file_settings(PG_FUNCTION_ARGS)
        per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
        oldcontext = MemoryContextSwitchTo(per_query_ctx);
 
-       /* Build a tuple descriptor for our result type */
-       tupdesc = CreateTemplateTupleDesc(NUM_PG_FILE_SETTINGS_ATTS);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 1, "sourcefile",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 2, "sourceline",
-                                          INT4OID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 3, "seqno",
-                                          INT4OID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 4, "name",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 5, "setting",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 6, "applied",
-                                          BOOLOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 7, "error",
-                                          TEXTOID, -1, 0);
-
        /* Build a tuplestore to return our results in */
        tupstore = tuplestore_begin_heap(true, false, work_mem);
        rsinfo->returnMode = SFRM_Materialize;
index 2dc875ebfbb64d1cf5601469e2f65bdf59e6cef1..e646a419106a92b68b1ca9e45cfdc946ad9da607 100644 (file)
@@ -26,14 +26,10 @@ pg_config(PG_FUNCTION_ARGS)
 {
        ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
        Tuplestorestate *tupstore;
-       HeapTuple       tuple;
        TupleDesc       tupdesc;
-       AttInMetadata *attinmeta;
-       MemoryContext per_query_ctx;
        MemoryContext oldcontext;
        ConfigData *configdata;
        size_t          configdata_len;
-       char       *values[2];
        int                     i = 0;
 
        /* check to see if caller supports us returning a tuplestore */
@@ -41,65 +37,38 @@ pg_config(PG_FUNCTION_ARGS)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("set-valued function called in context that cannot accept a set")));
-       if (!(rsinfo->allowedModes & SFRM_Materialize) ||
-               rsinfo->expectedDesc == NULL)
+       if (!(rsinfo->allowedModes & SFRM_Materialize))
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("materialize mode required, but it is not allowed in this context")));
 
-       per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
-       oldcontext = MemoryContextSwitchTo(per_query_ctx);
+       if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+               elog(ERROR, "return type must be a row type");
 
-       /* get the requested return tuple description */
-       tupdesc = CreateTupleDescCopy(rsinfo->expectedDesc);
+       /* Build tuplestore to hold the result rows */
+       oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
 
-       /*
-        * Check to make sure we have a reasonable tuple descriptor
-        */
-       if (tupdesc->natts != 2 ||
-               TupleDescAttr(tupdesc, 0)->atttypid != TEXTOID ||
-               TupleDescAttr(tupdesc, 1)->atttypid != TEXTOID)
-               ereport(ERROR,
-                               (errcode(ERRCODE_SYNTAX_ERROR),
-                                errmsg("query-specified return tuple and "
-                                               "function return type are not compatible")));
-
-       /* OK to use it */
-       attinmeta = TupleDescGetAttInMetadata(tupdesc);
-
-       /* let the caller know we're sending back a tuplestore */
+       tupstore = tuplestore_begin_heap(true, false, work_mem);
        rsinfo->returnMode = SFRM_Materialize;
+       rsinfo->setResult = tupstore;
+       rsinfo->setDesc = tupdesc;
 
-       /* initialize our tuplestore */
-       tupstore = tuplestore_begin_heap(true, false, work_mem);
+       MemoryContextSwitchTo(oldcontext);
 
        configdata = get_configdata(my_exec_path, &configdata_len);
        for (i = 0; i < configdata_len; i++)
        {
-               values[0] = configdata[i].name;
-               values[1] = configdata[i].setting;
-
-               tuple = BuildTupleFromCStrings(attinmeta, values);
-               tuplestore_puttuple(tupstore, tuple);
-       }
+               Datum           values[2];
+               bool            nulls[2];
 
-       /*
-        * no longer need the tuple descriptor reference created by
-        * TupleDescGetAttInMetadata()
-        */
-       ReleaseTupleDesc(tupdesc);
+               memset(values, 0, sizeof(values));
+               memset(nulls, 0, sizeof(nulls));
 
-       rsinfo->setResult = tupstore;
+               values[0] = CStringGetTextDatum(configdata[i].name);
+               values[1] = CStringGetTextDatum(configdata[i].setting);
 
-       /*
-        * SFRM_Materialize mode expects us to return a NULL Datum. The actual
-        * tuples are in our tuplestore and passed back through rsinfo->setResult.
-        * rsinfo->setDesc is set to the tuple description that we actually used
-        * to build our tuples with, so the caller can verify we did what it was
-        * expecting.
-        */
-       rsinfo->setDesc = tupdesc;
-       MemoryContextSwitchTo(oldcontext);
+               tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+       }
 
        return (Datum) 0;
 }
index 7885344164f8867a26dbcfa59ce18813f3efd998..21ad87c0241f433ca4955940d28fbc1e3258817e 100644 (file)
@@ -21,6 +21,7 @@
 #include "access/xact.h"
 #include "catalog/pg_type.h"
 #include "commands/portalcmds.h"
+#include "funcapi.h"
 #include "miscadmin.h"
 #include "storage/ipc.h"
 #include "utils/builtins.h"
@@ -1152,23 +1153,8 @@ pg_cursor(PG_FUNCTION_ARGS)
        per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
        oldcontext = MemoryContextSwitchTo(per_query_ctx);
 
-       /*
-        * build tupdesc for result tuples. This must match the definition of the
-        * pg_cursors view in system_views.sql
-        */
-       tupdesc = CreateTemplateTupleDesc(6);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 2, "statement",
-                                          TEXTOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 3, "is_holdable",
-                                          BOOLOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 4, "is_binary",
-                                          BOOLOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 5, "is_scrollable",
-                                          BOOLOID, -1, 0);
-       TupleDescInitEntry(tupdesc, (AttrNumber) 6, "creation_time",
-                                          TIMESTAMPTZOID, -1, 0);
+       if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+               elog(ERROR, "return type must be a row type");
 
        /*
         * We put all the tuples into a tuplestore in one scan of the hashtable.
@@ -1177,6 +1163,9 @@ pg_cursor(PG_FUNCTION_ARGS)
        tupstore =
                tuplestore_begin_heap(rsinfo->allowedModes & SFRM_Materialize_Random,
                                                          false, work_mem);
+       rsinfo->returnMode = SFRM_Materialize;
+       rsinfo->setResult = tupstore;
+       rsinfo->setDesc = tupdesc;
 
        /* generate junk in short-term context */
        MemoryContextSwitchTo(oldcontext);