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