Re: generating function default settings from pg_proc.dat

Hi,

On 2026-02-16 20:36:25 -0500, Tom Lane wrote:
> Here is a draft patch along those lines.  I've verified that
> the initial contents of pg_proc are exactly as before,
> except that json[b]_strip_nulls gain the correct provolatile
> value, and a few proargdefaults entries no longer involve
> cast functions.

Nice.


> @@ -817,8 +832,10 @@ $ perl  rewrite_dat_with_prokind.pl  pg_proc.dat
>        The following column types are supported directly by
>        <filename>bootstrap.c</filename>: <type>bool</type>,
>        <type>bytea</type>, <type>char</type> (1 byte),
> -      <type>name</type>, <type>int2</type>,
> -      <type>int4</type>, <type>regproc</type>, <type>regclass</type>,
> +      <type>int2</type>, <type>int4</type>, <type>int8</type>,
> +      <type>float4</type>, <type>float8</type>,
> +      <type>name</type>,
> +      <type>regproc</type>, <type>regclass</type>,
>        <type>regtype</type>, <type>text</type>,
>        <type>oid</type>, <type>tid</type>, <type>xid</type>,
>        <type>cid</type>, <type>int2vector</type>, <type>oidvector</type>,

Don't you also add jsonb support below?


> +	/*
> +	 * pg_node_tree values can't be inserted normally (pg_node_tree_in would
> +	 * just error out), so provide special cases for such columns that we
> +	 * would like to fill during bootstrap.
> +	 */
> +	if (typoid == PG_NODE_TREEOID)
> +	{
> +		/* pg_proc.proargdefaults */
> +		if (RelationGetRelid(boot_reldesc) == ProcedureRelationId &&
> +			i == Anum_pg_proc_proargdefaults - 1)
> +			values[i] = ConvertOneProargdefaultsValue(value);
> +		else					/* maybe other cases later */
> +			elog(ERROR, "can't handle pg_node_tree input");

Perhaps add the relid to the ERROR?


> +/* ----------------
> + *		ConvertOneProargdefaultsValue
> + *
> + * In general, proargdefaults can be a list of any expressions, but
> + * for bootstrap we only support a list of Const nodes.  The input
> + * has the form of a text array, and we feed non-null elements to the
> + * typinput functions for the appropriate parameters.
> + * ----------------
> + */
> +static Datum
> +ConvertOneProargdefaultsValue(char *value)
> +{
> +	int			pronargs;
> +	oidvector  *proargtypes;
> +	Datum		arrayval;
> +	Datum	   *array_datums;
> +	bool	   *array_nulls;
> +	int			array_count;
> +	List	   *proargdefaults;
> +
> +	/* The pg_proc columns we need to use must have been filled already */
> +	StaticAssertDecl(Anum_pg_proc_pronargs < Anum_pg_proc_proargdefaults,
> +					 "pronargs must come before proargdefaults");
> +	StaticAssertDecl(Anum_pg_proc_pronargdefaults < Anum_pg_proc_proargdefaults,
> +					 "pronargdefaults must come before proargdefaults");
> +	StaticAssertDecl(Anum_pg_proc_proargtypes < Anum_pg_proc_proargdefaults,
> +					 "proargtypes must come before proargdefaults");
> +	if (Nulls[Anum_pg_proc_pronargs - 1])
> +		elog(ERROR, "pronargs must not be null");
> +	if (Nulls[Anum_pg_proc_proargtypes - 1])
> +		elog(ERROR, "proargtypes must not be null");
> +	pronargs = DatumGetInt16(values[Anum_pg_proc_pronargs - 1]);
> +	proargtypes = DatumGetPointer(values[Anum_pg_proc_proargtypes - 1]);
> +	Assert(pronargs == proargtypes->dim1);
> +
> +	/* Parse the input string as a text[] value, then deconstruct to Datums */
> +	arrayval = OidFunctionCall3(F_ARRAY_IN,
> +								CStringGetDatum(value),
> +								ObjectIdGetDatum(TEXTOID),
> +								Int32GetDatum(-1));
>
> +	deconstruct_array_builtin(DatumGetArrayTypeP(arrayval), TEXTOID,
> +							  &array_datums, &array_nulls, &array_count);

If we convert to cstring below anyway, why not make it a cstring array?


> +	/* The values should correspond to the last N argtypes */
> +	if (array_count > pronargs)
> +		elog(ERROR, "too many proargdefaults entries");
> +
> +	/* Build the List of Const nodes */
> +	proargdefaults = NIL;
> +	for (int i = 0; i < array_count; i++)
> +	{
> +		Oid			argtype = proargtypes->values[pronargs - array_count + i];
> +		int16		typlen;
> +		bool		typbyval;
> +		char		typalign;
> +		char		typdelim;
> +		Oid			typioparam;
> +		Oid			typinput;
> +		Oid			typoutput;
> +		Oid			typcollation;
> +		Datum		defval;
> +		bool		defnull;
> +		Const	   *defConst;
> +
> +		boot_get_type_io_data(argtype,
> +							  &typlen, &typbyval, &typalign,
> +							  &typdelim, &typioparam,
> +							  &typinput, &typoutput);
> +		typcollation = boot_get_typcollation(argtype);
> +		defnull = array_nulls[i];
> +		if (defnull)
> +			defval = (Datum) 0;
> +		else
> +		{
> +			char	   *defstr = text_to_cstring(DatumGetTextPP(array_datums[i]));
> +
> +			defval = OidInputFunctionCall(typinput, defstr, typioparam, -1);
> +		}
> +
> +		defConst = makeConst(argtype,
> +							 -1,	/* never any typmod */
> +							 typcollation,
> +							 typlen,
> +							 defval,
> +							 defnull,
> +							 typbyval);
> +		proargdefaults = lappend(proargdefaults, defConst);
> +	}
> +
> +	/*
> +	 * Hack: fill in pronargdefaults with the right value.  This is surely
> +	 * ugly, but it beats making the programmer do it.
> +	 */
> +	values[Anum_pg_proc_pronargdefaults - 1] = Int16GetDatum(array_count);
> +	Nulls[Anum_pg_proc_pronargdefaults - 1] = false;

I don't mind the hack, but I wonder about it location. It's odd that the
caller puts the return value of ConvertOneProargdefaultsValue() into the
values array, but then ConvertOneProargdefaultsValue() also sets
pronargdefaults?


> +/* ----------------
> + *		boot_get_typcollation
> + *
> + * Obtain type's collation at bootstrap time.  This intentionally has
> + * the same API as lsyscache.c's get_typcollation.
> + *
> + * XXX would it be better to add another output to boot_get_type_io_data?

Yes, it seems like it would? There aren't many callers for it...


Greetings,

Andres Freund