From: Tomas Vondra Date: Thu, 4 Jul 2019 21:43:04 +0000 (+0200) Subject: Fix pg_mcv_list_items() to produce text[] X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4d66285adc6bb4f9e4fd394d478d663cbccb5fc8;p=postgresql 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 d9d78373a2..ba8ae9d6ba 100644 --- 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 190093ec1d..517aa32b39 100644 --- 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 87335248a0..36ae2ac9d8 100644 --- 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 9e770786cc..6a070a9649 100644 --- 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