Re: generating function default settings from pg_proc.dat - Mailing list pgsql-hackers

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