Fix pg_mcv_list_items() to produce text[]

The function pg_mcv_list_items() returns values stored in MCV items. The
items may contain columns with different data types, so the function was
generating text array-like representation, but in an ad-hoc way without
properly escaping various characters etc.

Fixed by simply building a text[] array, which also makes it easier to
use from queries etc.

Requires changes to pg_proc entry, so bump catversion.

Backpatch to 12, where multi-column MCV lists were introduced.

Author: Tomas Vondra
Reviewed-by: Dean Rasheed
Discussion: https://postgr.es/m/20190618205920.qtlzcu73whfpfqne@development

diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index d9d78373a226ee73f4eac99b8c1c93af3075c654..ba8ae9d6baab3d2803cbbf763b426dfdf0bafea8 100644 (file)
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1248,9 +1248,6 @@ Datum
 pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
 {
        FuncCallContext *funcctx;
-       int                     call_cntr;
-       int                     max_calls;
-       AttInMetadata *attinmeta;
 
        /* stuff done only on the first call of the function */
        if (SRF_IS_FIRSTCALL())
@@ -1280,13 +1277,13 @@ pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                         errmsg("function returning record called in context "
                                                        "that cannot accept type record")));
+               tupdesc = BlessTupleDesc(tupdesc);
 
                /*
                 * generate attribute metadata needed later to produce tuples from raw
                 * C strings
                 */
-               attinmeta = TupleDescGetAttInMetadata(tupdesc);
-               funcctx->attinmeta = attinmeta;
+               funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
 
                MemoryContextSwitchTo(oldcontext);
        }
@@ -1294,111 +1291,78 @@ pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
        /* stuff done on every call of the function */
        funcctx = SRF_PERCALL_SETUP();
 
-       call_cntr = funcctx->call_cntr;
-       max_calls = funcctx->max_calls;
-       attinmeta = funcctx->attinmeta;
-
-       if (call_cntr < max_calls)      /* do when there is more left to send */
+       if (funcctx->call_cntr < funcctx->max_calls)    /* do when there is more left to send */
        {
-               char      **values;
+               Datum           values[5];
+               bool            nulls[5];
                HeapTuple       tuple;
                Datum           result;
-
-               StringInfoData itemValues;
-               StringInfoData itemNulls;
+               ArrayBuildState *astate_values = NULL;
+               ArrayBuildState *astate_nulls = NULL;
 
                int                     i;
-
-               Oid                *outfuncs;
-               FmgrInfo   *fmgrinfo;
-
                MCVList    *mcvlist;
                MCVItem    *item;
 
                mcvlist = (MCVList *) funcctx->user_fctx;
 
-               Assert(call_cntr < mcvlist->nitems);
-
-               item = &mcvlist->items[call_cntr];
-
-               /*
-                * Prepare a values array for building the returned tuple. This should
-                * be an array of C strings which will be processed later by the type
-                * input functions.
-                */
-               values = (char **) palloc0(5 * sizeof(char *));
-
-               values[0] = (char *) palloc(64 * sizeof(char)); /* item index */
-               values[3] = (char *) palloc(64 * sizeof(char)); /* frequency */
-               values[4] = (char *) palloc(64 * sizeof(char)); /* base frequency */
+               Assert(funcctx->call_cntr < mcvlist->nitems);
 
-               outfuncs = (Oid *) palloc0(sizeof(Oid) * mcvlist->ndimensions);
-               fmgrinfo = (FmgrInfo *) palloc0(sizeof(FmgrInfo) * mcvlist->ndimensions);
+               item = &mcvlist->items[funcctx->call_cntr];
 
                for (i = 0; i < mcvlist->ndimensions; i++)
                {
-                       bool            isvarlena;
-
-                       getTypeOutputInfo(mcvlist->types[i], &outfuncs[i], &isvarlena);
-
-                       fmgr_info(outfuncs[i], &fmgrinfo[i]);
-               }
 
-               /* build the arrays of values / nulls */
-               initStringInfo(&itemValues);
-               initStringInfo(&itemNulls);
+                       astate_nulls = accumArrayResult(astate_nulls,
+                                                                 BoolGetDatum(item->isnull[i]),
+                                                                 false,
+                                                                 BOOLOID,
+                                                                 CurrentMemoryContext);
 
-               appendStringInfoChar(&itemValues, '{');
-               appendStringInfoChar(&itemNulls, '{');
-
-               for (i = 0; i < mcvlist->ndimensions; i++)
-               {
-                       Datum           val,
-                                               valout;
-
-                       if (i > 0)
+                       if (!item->isnull[i])
                        {
-                               appendStringInfoString(&itemValues, ", ");
-                               appendStringInfoString(&itemNulls, ", ");
+                               bool            isvarlena;
+                               Oid                     outfunc;
+                               FmgrInfo        fmgrinfo;
+                               Datum           val;
+                               text       *txt;
+
+                               /* lookup output func for the type */
+                               getTypeOutputInfo(mcvlist->types[i], &outfunc, &isvarlena);
+                               fmgr_info(outfunc, &fmgrinfo);
+
+                               val = FunctionCall1(&fmgrinfo, item->values[i]);
+                               txt = cstring_to_text(DatumGetPointer(val));
+
+                               astate_values = accumArrayResult(astate_values,
+                                                                 PointerGetDatum(txt),
+                                                                 false,
+                                                                 TEXTOID,
+                                                                 CurrentMemoryContext);
                        }
-
-                       if (item->isnull[i])
-                               valout = CStringGetDatum("NULL");
                        else
-                       {
-                               val = item->values[i];
-                               valout = FunctionCall1(&fmgrinfo[i], val);
-                       }
-
-                       appendStringInfoString(&itemValues, DatumGetCString(valout));
-                       appendStringInfoString(&itemNulls, item->isnull[i] ? "t" : "f");
+                               astate_values = accumArrayResult(astate_values,
+                                                                 (Datum) 0,
+                                                                 true,
+                                                                 TEXTOID,
+                                                                 CurrentMemoryContext);
                }
 
-               appendStringInfoChar(&itemValues, '}');
-               appendStringInfoChar(&itemNulls, '}');
-
-               snprintf(values[0], 64, "%d", call_cntr);
-               snprintf(values[3], 64, "%f", item->frequency);
-               snprintf(values[4], 64, "%f", item->base_frequency);
+               values[0] = Int32GetDatum(funcctx->call_cntr);
+               values[1] = PointerGetDatum(makeArrayResult(astate_values, CurrentMemoryContext));
+               values[2] = PointerGetDatum(makeArrayResult(astate_nulls, CurrentMemoryContext));
+               values[3] = Float8GetDatum(item->frequency);
+               values[4] = Float8GetDatum(item->base_frequency);
 
-               values[1] = itemValues.data;
-               values[2] = itemNulls.data;
+               /* no NULLs in the tuple */
+               memset(nulls, 0, sizeof(nulls));
 
                /* build a tuple */
-               tuple = BuildTupleFromCStrings(attinmeta, values);
+               tuple = heap_form_tuple(funcctx->attinmeta->tupdesc, values, nulls);
 
                /* make the tuple into a datum */
                result = HeapTupleGetDatum(tuple);
 
-               /* clean up (this is not really necessary) */
-               pfree(itemValues.data);
-               pfree(itemNulls.data);
-
-               pfree(values[0]);
-               pfree(values[3]);
-               pfree(values[4]);
-               pfree(values);
-
                SRF_RETURN_NEXT(funcctx, result);
        }
        else                                            /* do when there is no more left */

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 190093ec1ded15b41a43946597adf221ad5ec0dc..517aa32b39b67278e763864f8b33c70617c555ff 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201907041
+#define CATALOG_VERSION_NO     201907051
 
 #endif

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 87335248a03d5e1bb21a1bd0d8fa59ddc1e22cb7..36ae2ac9d8637843ac778d625362da7c19e41dbd 100644 (file)
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5019,7 +5019,7 @@
 { oid => '3427', descr => 'details about MCV list items',
   proname => 'pg_mcv_list_items', prorows => '1000', proretset => 't',
   provolatile => 's', prorettype => 'record', proargtypes => 'pg_mcv_list',
-  proallargtypes => '{pg_mcv_list,int4,text,_bool,float8,float8}',
+  proallargtypes => '{pg_mcv_list,int4,_text,_bool,float8,float8}',
   proargmodes => '{i,o,o,o,o,o}',
   proargnames => '{mcv_list,index,values,nulls,frequency,base_frequency}',
   prosrc => 'pg_stats_ext_mcvlist_items' },

diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 9e770786cc48d6ba1d0e632f7732341bf8cb277e..6a070a9649ddc4cd7b6e3781275d9c54dda6401d 100644 (file)
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -614,9 +614,9 @@ SELECT m.*
        pg_mcv_list_items(d.stxdmcv) m
  WHERE s.stxname = 'mcv_lists_stats'
    AND d.stxoid = s.oid;
- index |  values   |  nulls  | frequency | base_frequency 
--------+-----------+---------+-----------+----------------
-     0 | {1, 2, 3} | {f,f,f} |         1 |              1
+ index | values  |  nulls  | frequency | base_frequency 
+-------+---------+---------+-----------+----------------
+     0 | {1,2,3} | {f,f,f} |         1 |              1
 (1 row)
 
 -- mcv with arrays