pgsql: Simplify creation of built-in functions with default arguments. - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Simplify creation of built-in functions with default arguments.
Date
Msg-id E1vsn0i-0000d8-0f@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Simplify creation of built-in functions with default arguments.

Up to now, to create such a function, one had to make a pg_proc.dat
entry and then overwrite it with a CREATE OR REPLACE command in
system_functions.sql.  That's error-prone (cf. bug #19409) and
results in leaving dead rows in the initial contents of pg_proc.

Manual maintenance of pg_node_tree strings seems entirely impractical,
and parsing expressions during bootstrap would be extremely difficult
as well.  But Andres Freund observed that all the current use-cases
are simple constants, and building a Const node is well within the
capabilities of bootstrap mode.  So this patch invents a special case:
if bootstrap mode is asked to ingest a non-null value for
pg_proc.proargdefaults (which would otherwise fail in
pg_node_tree_in), it parses the value as an array literal and then
feeds the element strings to the input functions for the corresponding
parameter types.  Then we can build a suitable pg_node_tree string
with just a few more lines of code.

This allows removing all the system_functions.sql entries that are
just there to set up default arguments, replacing them with
proargdefaults fields in pg_proc.dat entries.  The old technique
remains available in case someone needs a non-constant default.

The initial contents of pg_proc are demonstrably the same after
this patch, except that (1) json_strip_nulls and jsonb_strip_nulls
now have the correct provolatile setting, as per bug #19409;
(2) pg_terminate_backend, make_interval, and drandom_normal
now have defaults that don't include a type coercion, which is
how they should have been all along.

In passing, remove some unused entries from bootstrap.c's TypInfo[]
array.  I had to add some new ones because we'll now need an entry for
each default-possessing system function parameter, but we shouldn't
carry more than we need there; it's just a maintenance gotcha.

Bug: #19409
Reported-by: Lucio Chiessi <lucio.chiessi@trustly.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Author: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/183292bb-4891-4c96-a3ca-e78b5e0e1358@dunslane.net
Discussion: https://postgr.es/m/19409-e16cd2605e59a4af@postgresql.org

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/759b03b24ce96f0ba6d734b570d1a6f4a0fb1177

Modified Files
--------------
doc/src/sgml/bki.sgml                    |  34 +++-
src/backend/bootstrap/bootstrap.c        | 177 ++++++++++++++++---
src/backend/catalog/system_functions.sql | 283 +------------------------------
src/backend/utils/cache/lsyscache.c      |   4 +-
src/include/bootstrap/bootstrap.h        |   3 +-
src/include/catalog/catversion.h         |   2 +-
src/include/catalog/pg_proc.dat          |  86 ++++++++--
7 files changed, 263 insertions(+), 326 deletions(-)


pgsql-committers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: pgsql: Use standard die() handler for SIGTERM in bgworkers
Next
From: Michael Paquier
Date:
Subject: pgsql: Sanitize some WAL-logging buffer handling in GIN and GiST code