Re: updates for handling optional argument in system functions

Hi!

On Wed, Sep 16, 2026 at 12:33:18AM +0800, Rui Zhao wrote:
> Hi Mark,
>
> v9 applies cleanly on current master (f7700080dc4), and make check, the
> TAP suites under src/bin and contrib check pass here. No objections to
> the code; what follows is what I verified and a few small things.
>
> 1. Every existing call form behaves the same as before. I ran one
> script against an unpatched build and a v9 build of the same base
> commit: the one-, two- and three-argument calls of the six functions
> (pg_get_viewdef by name, by oid and with wrap_column included, plus the
> unknown-literal and NULL cases and the "is not unique" error for
> pg_get_viewdef(oid, NULL)), pg_indexes/pg_rules/pg_views,
> information_schema.columns and check_constraints, \d output and
> schema-only pg_dump output are byte-identical between the two builds.
> Views defined over the one-argument forms deparse identically as well:
> the parser doesn't store default arguments in the FuncExpr, so
> pg_get_viewdef of such a view still prints pg_get_viewdef(c.oid). What
> is new is pg_get_indexdef(index, column) with just two arguments, and
> named notation such as pg_get_viewdef(view => 'v', pretty => true).
>
> 2. pg_upgrade from an unpatched cluster works for views over these
> functions: afterwards the view trees reference pg_proc OIDs
> 2504/2505/2506 instead of 1573/1640/1641, and the views return the same
> rows as before the upgrade. For the case Fujii described, a GRANT on
> pg_get_ruledef(oid) in the old cluster, pg_upgrade --check reports
> "Clusters are compatible" and the upgrade then fails in the restore
> step:
>
> pg_restore: error: could not execute query: ERROR: function
> pg_catalog.pg_get_ruledef(oid) does not exist
> Command was: REVOKE ALL ON FUNCTION
> "pg_catalog"."pg_get_ruledef"("oid") FROM PUBLIC;
>
> pg_dump only emits ACLs for pg_catalog functions that differ from
> pg_init_privs, so this needs someone to have changed the privileges of
> one of the removed signatures.

Right. Do I understand correctly that this is going to be considered an
unusual and rare occurrence? That we will expect the user to manually
handle the privileges instead of having pg_upgrade recognize that.

> 3. Smaller things:
>
> * The six commit subjects still say "in system_functions.sql"; since v4
> the patches set proargdefaults in pg_proc.dat and don't touch
> system_functions.sql.

Oops, new version coming in an attempt to correct that.

> * reformat_dat_file.pl moves descr back onto the "{ oid =>" line for
> 2505, 2506, 2507 and 2509 now that the descriptions are shorter. (It
> also reformats uuid_larger/uuid_smaller, but that is pre-existing on
> master.)
>
> * func-info.sgml has pg_get_indexdef ( index oid [, column integer,
> pretty boolean ] ); with the defaults, column can be given without
> pretty, so [, column integer [, pretty boolean ] ] would describe it.

I think I fixed that now.

> * For C callers: the _ext symbols go away and the fmgroids.h macros
> lose their type suffixes (F_PG_GET_EXPR_PG_NODE_TREE_OID_BOOL becomes
> F_PG_GET_EXPR). A DirectFunctionCall2(pg_get_expr, ...) in extension
> code still compiles and then reads past the end of its
> FunctionCallInfo, which is what the sanitizer caught in tablecmds.c
> in v1.

I may need a reminder, or help understanding this better. I believe
nothing in the core code has been hard coded to use the generated
macros. I recognize that there may be other code that could use the
previous macros, but that would be situations where the code would have
been built against an older major version and we say in the docs
"Cautious users will want to test their client applications on the new
version before switching over fully..." in doc/src/sgml/runtime.sgml.
Am I misunderstanding?

In the meantime, I've attached v10.

Thanks for the review!

Regards,
Mark
--
Mark Wong
pgEdge: https://www.pgedge.com