public inbox for [email protected]
help / color / mirror / Atom feedupdates for handling optional argument in system functions
18+ messages / 6 participants
[nested] [flat]
* updates for handling optional argument in system functions
@ 2025-12-09 23:28 Mark Wong <[email protected]>
0 siblings, 2 replies; 18+ messages in thread
From: Mark Wong @ 2025-12-09 23:28 UTC (permalink / raw)
To: [email protected]
Hi everyone,
I noticed how it was preferred to define optional arguments with the
system functions in system_functions.sql instead of defining them in
pg_proc.dat.
I took a gross stab at updating the ones that ended in _ext, which
turned out to be 7 declarations across 6 system functions, and created a
patch per system function, hoping it would be easier to review.
Perhaps the most interesting thing to share is the total reduction of
the lines of code, although system_functions.sql only grows:
src/backend/catalog/system_functions.sql | 49 ++++++++
src/backend/utils/adt/ruleutils.c | 130 ----------------------
src/include/catalog/pg_proc.dat | 36 ++----
3 files changed, 56 insertions(+), 159 deletions(-)
Is that something we want?
Regards,
Mark
--
Mark Wong <[email protected]>
EDB https://enterprisedb.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2025-12-10 09:33 Daniel Gustafsson <[email protected]>
parent: Mark Wong <[email protected]>
1 sibling, 0 replies; 18+ messages in thread
From: Daniel Gustafsson @ 2025-12-10 09:33 UTC (permalink / raw)
To: Mark Wong <[email protected]>; +Cc: Pgsql Hackers <[email protected]>
> On 10 Dec 2025, at 00:28, Mark Wong <[email protected]> wrote:
> Is that something we want?
I have yet to study the patch, but conceptually I am favour of this. I find
reading the code is easier when it's done this way.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-03-12 03:28 Mark Wong <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Mark Wong @ 2026-03-12 03:28 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: Chao Li <[email protected]>; [email protected]
On Tue, Mar 03, 2026 at 05:23:58PM +0100, Álvaro Herrera wrote:
> On 2026-Jan-29, Mark Wong wrote:
>
> > I don't have a solution for the case of a view storing the OID, but Álvaro
> > Herrera suggested to me to at least try preventing those OIDs from being
> > reused.
> >
> > I've attached a v3 patch set that introduces src/include/catalog/pg_retired.dat
> > to store previously used OIDs and procedure names that the scripts unused_oids
> > and renumber_oids.pl can consume to prevent the reuse of retired OIDs.
>
> Thinking about this again, I wonder where did we get the idea that
> reusing OIDs would be a problem. How exactly would this happen? When
> you pg_upgrade, your views are taken from a `pg_dump --binary-upgrade`
> of the original server, and then recreated using the text
> representation of the DDL. We don't pass the function OIDs in any way
> from the old server to the new server. And there's no other way (than
> pg_upgrade) to go from one major version to the next one where the OID
> has been reused.
>
> So why did we think this was an actual problem?
I'm not sure. I think I see the OID of a function get used in some places
likes rewriteDefine.c and parse_funcs.c, but I'm not sure if I see a function
OID get written out and re-read for a function name lookup in an upgrade code
path, yet...
Regards,
Mark
--
Mark Wong <[email protected]>
EDB https://enterprisedb.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-02 18:36 Mark Wong <[email protected]>
parent: Mark Wong <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Mark Wong @ 2026-04-02 18:36 UTC (permalink / raw)
To: [email protected]
Hi everyone,
I've attached v5, simply a needed rebase due to some other churn in
pg_proc.dat.
Regards,
Mark
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-07 22:26 Andreas Karlsson <[email protected]>
parent: Mark Wong <[email protected]>
0 siblings, 2 replies; 18+ messages in thread
From: Andreas Karlsson @ 2026-04-07 22:26 UTC (permalink / raw)
To: Mark Wong <[email protected]>; [email protected]
On 4/2/26 8:36 PM, Mark Wong wrote:
> I've attached v5, simply a needed rebase due to some other churn in
> pg_proc.dat.
Nice, I like the patch. The code changes looks good and I like the
removal of these duplicate functions and use of default arguments. But I
think the names of the arguments should be aligned with the names we
have in the documentation. And that may mean that we should change the
documentation.
For example:
= Code
pg_get_ruledef(rule, pretty)
= Docs
pg_get_ruledef(rule_oid, pretty_bool)
= Should docs maybe be updated to the following?
pg_get_ruledef(rule oid, pretty bool)
--
Andreas Karlsson
Percona
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-07 23:28 Andreas Karlsson <[email protected]>
parent: Andreas Karlsson <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Andreas Karlsson @ 2026-04-07 23:28 UTC (permalink / raw)
To: Mark Wong <[email protected]>; [email protected]
On 4/8/26 12:26 AM, Andreas Karlsson wrote:
> On 4/2/26 8:36 PM, Mark Wong wrote:
>> I've attached v5, simply a needed rebase due to some other churn in
>> pg_proc.dat.
>
> Nice, I like the patch. The code changes looks good and I like the
> removal of these duplicate functions and use of default arguments. But I
> think the names of the arguments should be aligned with the names we
> have in the documentation. And that may mean that we should change the
> documentation.
Forgot to attach rebased patches.
--
Andreas Karlsson
Percona
Attachments:
[text/x-patch] v6-0001-Handle-pg_get_ruledef-default-args-in-system_func.patch (2.5K, 2-v6-0001-Handle-pg_get_ruledef-default-args-in-system_func.patch)
download | inline diff:
From 9c24ecf0bb50d9e7ca18da532b9765fd521659c2 Mon Sep 17 00:00:00 2001
From: Mark Wong <[email protected]>
Date: Mon, 8 Dec 2025 15:41:07 -0800
Subject: [PATCH v6 1/6] Handle pg_get_ruledef default args in
system_functions.sql
Modernize pg_get_ruledef to use proargdefaults for optional pretty
argument.
---
src/backend/utils/adt/ruleutils.c | 18 ------------------
src/include/catalog/pg_proc.dat | 6 ++----
2 files changed, 2 insertions(+), 22 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 35083fcc733..6a4c7e4822d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -569,24 +569,6 @@ static void get_json_table_nested_columns(TableFunc *tf, JsonTablePlan *plan,
*/
Datum
pg_get_ruledef(PG_FUNCTION_ARGS)
-{
- Oid ruleoid = PG_GETARG_OID(0);
- int prettyFlags;
- char *res;
-
- prettyFlags = PRETTYFLAG_INDENT;
-
- res = pg_get_ruledef_worker(ruleoid, prettyFlags);
-
- if (res == NULL)
- PG_RETURN_NULL();
-
- PG_RETURN_TEXT_P(string_to_text(res));
-}
-
-
-Datum
-pg_get_ruledef_ext(PG_FUNCTION_ARGS)
{
Oid ruleoid = PG_GETARG_OID(0);
bool pretty = PG_GETARG_BOOL(1);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 99fa9a6ede2..c007d92b7b0 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3981,9 +3981,6 @@
{ oid => '8302', descr => 'source text of a property graph',
proname => 'pg_get_propgraphdef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_propgraphdef' },
-{ oid => '1573', descr => 'source text of a rule',
- proname => 'pg_get_ruledef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid', prosrc => 'pg_get_ruledef' },
{ oid => '1640', descr => 'select statement of a view',
proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
prorettype => 'text', proargtypes => 'text',
@@ -8593,7 +8590,8 @@
# System-view support functions with pretty-print option
{ oid => '2504', descr => 'source text of a rule with pretty-print option',
proname => 'pg_get_ruledef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid bool', prosrc => 'pg_get_ruledef_ext' },
+ proargtypes => 'oid bool', proargnames => '{rule,pretty}',
+ proargdefaults => '{false}',prosrc => 'pg_get_ruledef' },
{ oid => '2505',
descr => 'select statement of a view with pretty-print option',
proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
--
2.47.3
[text/x-patch] v6-0002-Handle-pg_get_viewdef-default-args-in-system_func.patch (3.9K, 3-v6-0002-Handle-pg_get_viewdef-default-args-in-system_func.patch)
download | inline diff:
From 7a2c0450bd31392a0124e425edd6cb347b63ec0e Mon Sep 17 00:00:00 2001
From: Mark Wong <[email protected]>
Date: Tue, 9 Dec 2025 09:33:21 -0800
Subject: [PATCH v6 2/6] Handle pg_get_viewdef default args in
system_functions.sql
Modernize pg_get_viewdef to use proargdefaults to handle the optional
pretty argument for both versions that use OID or view name.
---
src/backend/utils/adt/ruleutils.c | 44 -------------------------------
src/include/catalog/pg_proc.dat | 13 +++------
2 files changed, 4 insertions(+), 53 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 6a4c7e4822d..1202cb07c07 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -669,25 +669,6 @@ pg_get_ruledef_worker(Oid ruleoid, int prettyFlags)
*/
Datum
pg_get_viewdef(PG_FUNCTION_ARGS)
-{
- /* By OID */
- Oid viewoid = PG_GETARG_OID(0);
- int prettyFlags;
- char *res;
-
- prettyFlags = PRETTYFLAG_INDENT;
-
- res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
-
- if (res == NULL)
- PG_RETURN_NULL();
-
- PG_RETURN_TEXT_P(string_to_text(res));
-}
-
-
-Datum
-pg_get_viewdef_ext(PG_FUNCTION_ARGS)
{
/* By OID */
Oid viewoid = PG_GETARG_OID(0);
@@ -727,31 +708,6 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
Datum
pg_get_viewdef_name(PG_FUNCTION_ARGS)
-{
- /* By qualified name */
- text *viewname = PG_GETARG_TEXT_PP(0);
- int prettyFlags;
- RangeVar *viewrel;
- Oid viewoid;
- char *res;
-
- prettyFlags = PRETTYFLAG_INDENT;
-
- /* Look up view name. Can't lock it - we might not have privileges. */
- viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
- viewoid = RangeVarGetRelid(viewrel, NoLock, false);
-
- res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
-
- if (res == NULL)
- PG_RETURN_NULL();
-
- PG_RETURN_TEXT_P(string_to_text(res));
-}
-
-
-Datum
-pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
{
/* By qualified name */
text *viewname = PG_GETARG_TEXT_PP(0);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index c007d92b7b0..d9e52018bce 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3981,13 +3981,6 @@
{ oid => '8302', descr => 'source text of a property graph',
proname => 'pg_get_propgraphdef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_propgraphdef' },
-{ oid => '1640', descr => 'select statement of a view',
- proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
- prorettype => 'text', proargtypes => 'text',
- prosrc => 'pg_get_viewdef_name' },
-{ oid => '1641', descr => 'select statement of a view',
- proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
- prorettype => 'text', proargtypes => 'oid', prosrc => 'pg_get_viewdef' },
{ oid => '1642', descr => 'role name by OID (with fallback)',
proname => 'pg_get_userbyid', provolatile => 's', prorettype => 'name',
proargtypes => 'oid', prosrc => 'pg_get_userbyid' },
@@ -8596,12 +8589,14 @@
descr => 'select statement of a view with pretty-print option',
proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
prorettype => 'text', proargtypes => 'text bool',
- prosrc => 'pg_get_viewdef_name_ext' },
+ proargnames => '{view,pretty}', proargdefaults => '{false}',
+ prosrc => 'pg_get_viewdef_name' },
{ oid => '2506',
descr => 'select statement of a view with pretty-print option',
proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
prorettype => 'text', proargtypes => 'oid bool',
- prosrc => 'pg_get_viewdef_ext' },
+ proargnames => '{view,pretty}', proargdefaults => '{false}',
+ prosrc => 'pg_get_viewdef' },
{ oid => '3159',
descr => 'select statement of a view with pretty-printing and specified line wrapping',
proname => 'pg_get_viewdef', provolatile => 's', proparallel => 'r',
--
2.47.3
[text/x-patch] v6-0003-Handle-pg_get_indexdef-default-args-in-system_fun.patch (2.6K, 4-v6-0003-Handle-pg_get_indexdef-default-args-in-system_fun.patch)
download | inline diff:
From 74aff0343c08fca248b76665804540c07c37c7c3 Mon Sep 17 00:00:00 2001
From: Mark Wong <[email protected]>
Date: Tue, 9 Dec 2025 10:02:15 -0800
Subject: [PATCH v6 3/6] Handle pg_get_indexdef default args in
system_functions.sql
Modernize pg_get_indexdef to use proargdefaults to handle the optional
column and pretty argument.
---
src/backend/utils/adt/ruleutils.c | 20 --------------------
src/include/catalog/pg_proc.dat | 6 ++----
2 files changed, 2 insertions(+), 24 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 1202cb07c07..55926ec15e0 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1125,26 +1125,6 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
*/
Datum
pg_get_indexdef(PG_FUNCTION_ARGS)
-{
- Oid indexrelid = PG_GETARG_OID(0);
- int prettyFlags;
- char *res;
-
- prettyFlags = PRETTYFLAG_INDENT;
-
- res = pg_get_indexdef_worker(indexrelid, 0, NULL,
- false, false,
- false, false,
- prettyFlags, true);
-
- if (res == NULL)
- PG_RETURN_NULL();
-
- PG_RETURN_TEXT_P(string_to_text(res));
-}
-
-Datum
-pg_get_indexdef_ext(PG_FUNCTION_ARGS)
{
Oid indexrelid = PG_GETARG_OID(0);
int32 colno = PG_GETARG_INT32(1);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d9e52018bce..18b62310922 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3984,9 +3984,6 @@
{ oid => '1642', descr => 'role name by OID (with fallback)',
proname => 'pg_get_userbyid', provolatile => 's', prorettype => 'name',
proargtypes => 'oid', prosrc => 'pg_get_userbyid' },
-{ oid => '1643', descr => 'index description',
- proname => 'pg_get_indexdef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid', prosrc => 'pg_get_indexdef' },
{ oid => '3415', descr => 'extended statistics object description',
proname => 'pg_get_statisticsobjdef', provolatile => 's',
prorettype => 'text', proargtypes => 'oid',
@@ -8605,7 +8602,8 @@
{ oid => '2507',
descr => 'index description (full create statement or single expression) with pretty-print option',
proname => 'pg_get_indexdef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid int4 bool', prosrc => 'pg_get_indexdef_ext' },
+ proargtypes => 'oid int4 bool', proargnames => '{index,column,pretty}',
+ proargdefaults => '{0,false}', prosrc => 'pg_get_indexdef' },
{ oid => '2508', descr => 'constraint description with pretty-print option',
proname => 'pg_get_constraintdef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid bool', prosrc => 'pg_get_constraintdef_ext' },
--
2.47.3
[text/x-patch] v6-0004-Handle-pg_get_constraintdef-default-args-in-syste.patch (2.6K, 5-v6-0004-Handle-pg_get_constraintdef-default-args-in-syste.patch)
download | inline diff:
From 85e4ac83e5091fcace385ae389aaa2680b7ba4d6 Mon Sep 17 00:00:00 2001
From: Mark Wong <[email protected]>
Date: Tue, 9 Dec 2025 10:59:41 -0800
Subject: [PATCH v6 4/6] Handle pg_get_constraintdef default args in
system_functions.sql
Modernize pg_get_constraintdef to use proargdefaults to handle the
optional pretty argument.
---
src/backend/utils/adt/ruleutils.c | 17 -----------------
src/include/catalog/pg_proc.dat | 6 ++----
2 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 55926ec15e0..2d39bce7fd7 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2420,23 +2420,6 @@ pg_get_partconstrdef_string(Oid partitionId, char *aliasname)
*/
Datum
pg_get_constraintdef(PG_FUNCTION_ARGS)
-{
- Oid constraintId = PG_GETARG_OID(0);
- int prettyFlags;
- char *res;
-
- prettyFlags = PRETTYFLAG_INDENT;
-
- res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true);
-
- if (res == NULL)
- PG_RETURN_NULL();
-
- PG_RETURN_TEXT_P(string_to_text(res));
-}
-
-Datum
-pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
{
Oid constraintId = PG_GETARG_OID(0);
bool pretty = PG_GETARG_BOOL(1);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 18b62310922..657a2a7d4f8 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4006,9 +4006,6 @@
{ oid => '1662', descr => 'trigger description',
proname => 'pg_get_triggerdef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_triggerdef' },
-{ oid => '1387', descr => 'constraint description',
- proname => 'pg_get_constraintdef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid', prosrc => 'pg_get_constraintdef' },
{ oid => '1716', descr => 'deparse an encoded expression',
proname => 'pg_get_expr', provolatile => 's', prorettype => 'text',
proargtypes => 'pg_node_tree oid', prosrc => 'pg_get_expr' },
@@ -8606,7 +8603,8 @@
proargdefaults => '{0,false}', prosrc => 'pg_get_indexdef' },
{ oid => '2508', descr => 'constraint description with pretty-print option',
proname => 'pg_get_constraintdef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid bool', prosrc => 'pg_get_constraintdef_ext' },
+ proargtypes => 'oid bool', proargnames => '{constraint,pretty}',
+ proargdefaults => '{false}', prosrc => 'pg_get_constraintdef' },
{ oid => '8760', descr => 'get DDL to recreate a role',
proname => 'pg_get_role_ddl', provariadic => 'text', proisstrict => 'f',
provolatile => 's', proretset => 't', prorows => '10', prorettype => 'text',
--
2.47.3
[text/x-patch] v6-0005-Handle-pg_get_expr-default-args-in-system_functio.patch (3.3K, 6-v6-0005-Handle-pg_get_expr-default-args-in-system_functio.patch)
download | inline diff:
From 9c29e0ce8722c5df15eb5f09c3cd36d1966ce395 Mon Sep 17 00:00:00 2001
From: Mark Wong <[email protected]>
Date: Tue, 9 Dec 2025 11:17:56 -0800
Subject: [PATCH v6 5/6] Handle pg_get_expr default args in
system_functions.sql
Modernize pg_get_expr to use proargdefaults to handle the optional
pretty argument. That also means any direct function calls now needs to
set the pretty parameter.
---
src/backend/commands/tablecmds.c | 5 +++--
src/backend/utils/adt/ruleutils.c | 17 -----------------
src/include/catalog/pg_proc.dat | 7 +++----
3 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index eec09ba1ded..73eb6322daf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -17695,8 +17695,9 @@ decompile_conbin(HeapTuple contup, TupleDesc tupdesc)
if (isnull)
elog(ERROR, "null conbin for constraint %u", con->oid);
- expr = DirectFunctionCall2(pg_get_expr, attr,
- ObjectIdGetDatum(con->conrelid));
+ expr = DirectFunctionCall3(pg_get_expr, attr,
+ ObjectIdGetDatum(con->conrelid),
+ BoolGetDatum(false));
return TextDatumGetCString(expr);
}
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 2d39bce7fd7..707f83d4310 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2932,23 +2932,6 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
*/
Datum
pg_get_expr(PG_FUNCTION_ARGS)
-{
- text *expr = PG_GETARG_TEXT_PP(0);
- Oid relid = PG_GETARG_OID(1);
- text *result;
- int prettyFlags;
-
- prettyFlags = PRETTYFLAG_INDENT;
-
- result = pg_get_expr_worker(expr, relid, prettyFlags);
- if (result)
- PG_RETURN_TEXT_P(result);
- else
- PG_RETURN_NULL();
-}
-
-Datum
-pg_get_expr_ext(PG_FUNCTION_ARGS)
{
text *expr = PG_GETARG_TEXT_PP(0);
Oid relid = PG_GETARG_OID(1);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 657a2a7d4f8..56d82af5c91 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4006,9 +4006,6 @@
{ oid => '1662', descr => 'trigger description',
proname => 'pg_get_triggerdef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_triggerdef' },
-{ oid => '1716', descr => 'deparse an encoded expression',
- proname => 'pg_get_expr', provolatile => 's', prorettype => 'text',
- proargtypes => 'pg_node_tree oid', prosrc => 'pg_get_expr' },
{ oid => '1665', descr => 'name of sequence for a serial column',
proname => 'pg_get_serial_sequence', provolatile => 's', prorettype => 'text',
proargtypes => 'text text', prosrc => 'pg_get_serial_sequence' },
@@ -8640,7 +8637,9 @@
{ oid => '2509',
descr => 'deparse an encoded expression with pretty-print option',
proname => 'pg_get_expr', provolatile => 's', prorettype => 'text',
- proargtypes => 'pg_node_tree oid bool', prosrc => 'pg_get_expr_ext' },
+ proargtypes => 'pg_node_tree oid bool',
+ proargnames => '{expr,relation,pretty}', proargdefaults => '{false}',
+ prosrc => 'pg_get_expr' },
{ oid => '2510', descr => 'get the prepared statements for this session',
proname => 'pg_prepared_statement', prorows => '1000', proretset => 't',
provolatile => 's', proparallel => 'r', prorettype => 'record',
--
2.47.3
[text/x-patch] v6-0006-Handle-pg_get_triggerdef-default-args-in-system_f.patch (2.3K, 7-v6-0006-Handle-pg_get_triggerdef-default-args-in-system_f.patch)
download | inline diff:
From af5851d73197a582aa1199961f17dc5a8ba7581a Mon Sep 17 00:00:00 2001
From: Mark Wong <[email protected]>
Date: Tue, 9 Dec 2025 11:51:39 -0800
Subject: [PATCH v6 6/6] Handle pg_get_triggerdef default args in
system_functions.sql
Modernize pg_get_triggerdef to use proargdefaults to handle the optional
pretty argument.
---
src/backend/utils/adt/ruleutils.c | 14 --------------
src/include/catalog/pg_proc.dat | 6 ++----
2 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 707f83d4310..249f4518f62 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -818,20 +818,6 @@ pg_get_viewdef_worker(Oid viewoid, int prettyFlags, int wrapColumn)
*/
Datum
pg_get_triggerdef(PG_FUNCTION_ARGS)
-{
- Oid trigid = PG_GETARG_OID(0);
- char *res;
-
- res = pg_get_triggerdef_worker(trigid, false);
-
- if (res == NULL)
- PG_RETURN_NULL();
-
- PG_RETURN_TEXT_P(string_to_text(res));
-}
-
-Datum
-pg_get_triggerdef_ext(PG_FUNCTION_ARGS)
{
Oid trigid = PG_GETARG_OID(0);
bool pretty = PG_GETARG_BOOL(1);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 56d82af5c91..1d304d1e316 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4003,9 +4003,6 @@
proname => 'pg_get_partition_constraintdef', provolatile => 's',
prorettype => 'text', proargtypes => 'oid',
prosrc => 'pg_get_partition_constraintdef' },
-{ oid => '1662', descr => 'trigger description',
- proname => 'pg_get_triggerdef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid', prosrc => 'pg_get_triggerdef' },
{ oid => '1665', descr => 'name of sequence for a serial column',
proname => 'pg_get_serial_sequence', provolatile => 's', prorettype => 'text',
proargtypes => 'text text', prosrc => 'pg_get_serial_sequence' },
@@ -8675,7 +8672,8 @@
prosrc => 'pg_timezone_names' },
{ oid => '2730', descr => 'trigger description with pretty-print option',
proname => 'pg_get_triggerdef', provolatile => 's', prorettype => 'text',
- proargtypes => 'oid bool', prosrc => 'pg_get_triggerdef_ext' },
+ proargtypes => 'oid bool', proargnames => '{trigger,pretty}',
+ proargdefaults => '{false}', prosrc => 'pg_get_triggerdef' },
# asynchronous notifications
{ oid => '3035',
--
2.47.3
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-07 23:42 Mark Wong <[email protected]>
parent: Andreas Karlsson <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Mark Wong @ 2026-04-07 23:42 UTC (permalink / raw)
To: Andreas Karlsson <[email protected]>; +Cc: [email protected]
On Wed, Apr 08, 2026 at 12:26:05AM +0200, Andreas Karlsson wrote:
> On 4/2/26 8:36 PM, Mark Wong wrote:
> > I've attached v5, simply a needed rebase due to some other churn in
> > pg_proc.dat.
>
> Nice, I like the patch. The code changes looks good and I like the
> removal of these duplicate functions and use of default arguments. But I
> think the names of the arguments should be aligned with the names we
> have in the documentation. And that may mean that we should change the
> documentation.
Thanks!
> For example:
>
> = Code
>
> pg_get_ruledef(rule, pretty)
>
> = Docs
>
> pg_get_ruledef(rule_oid, pretty_bool)
>
> = Should docs maybe be updated to the following?
>
> pg_get_ruledef(rule oid, pretty bool)
I agree with the aligning the names, but maybe I was looking in a
different place?
In the doc/src/sgml/func/func-info.sgml (a4f774cf1c7e) I think I see
pg_get_ruledef(rule oid, pretty bool) already:
<function>pg_get_ruledef</function> ( <parameter>rule</parameter> <type>oid</type> <optional>, <parameter>pretty</parameter> <type>boolean</type> </optional> )
I don't see any matches when I grep for rule_oid or pretty_bool...
Regards,
Mark
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-07 23:44 Mark Wong <[email protected]>
parent: Andreas Karlsson <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Mark Wong @ 2026-04-07 23:44 UTC (permalink / raw)
To: Andreas Karlsson <[email protected]>; +Cc: [email protected]
On Wed, Apr 08, 2026 at 01:28:29AM +0200, Andreas Karlsson wrote:
> On 4/8/26 12:26 AM, Andreas Karlsson wrote:
> > On 4/2/26 8:36 PM, Mark Wong wrote:
> >> I've attached v5, simply a needed rebase due to some other churn in
> >> pg_proc.dat.
> >
> > Nice, I like the patch. The code changes looks good and I like the
> > removal of these duplicate functions and use of default arguments. But I
> > think the names of the arguments should be aligned with the names we
> > have in the documentation. And that may mean that we should change the
> > documentation.
> Forgot to attach rebased patches.
Another rebase needed so quickly? :) Thanks for the assist!
Regards,
Mark
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-07 23:51 Andreas Karlsson <[email protected]>
parent: Mark Wong <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Karlsson @ 2026-04-07 23:51 UTC (permalink / raw)
To: Mark Wong <[email protected]>; +Cc: [email protected]
On 4/8/26 1:42 AM, Mark Wong wrote:
> I don't see any matches when I grep for rule_oid or pretty_bool...
Sorry for the noise, I was looking at an old version of the docs. The
parameters do indeed match the docs. But now that I looked at the
patches again I found a real issue which I think should be fixed.
I think the following:
"select statement of a view with pretty-print option"
should likely just be:
"select statement of a view"
Andreas
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-08 01:33 Chao Li <[email protected]>
parent: Mark Wong <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Chao Li @ 2026-04-08 01:33 UTC (permalink / raw)
To: Mark Wong <[email protected]>; +Cc: Andreas Karlsson <[email protected]>; [email protected]
> On Apr 8, 2026, at 07:44, Mark Wong <[email protected]> wrote:
>
> On Wed, Apr 08, 2026 at 01:28:29AM +0200, Andreas Karlsson wrote:
>> On 4/8/26 12:26 AM, Andreas Karlsson wrote:
>>> On 4/2/26 8:36 PM, Mark Wong wrote:
>>>> I've attached v5, simply a needed rebase due to some other churn in
>>>> pg_proc.dat.
>>>
>>> Nice, I like the patch. The code changes looks good and I like the
>>> removal of these duplicate functions and use of default arguments. But I
>>> think the names of the arguments should be aligned with the names we
>>> have in the documentation. And that may mean that we should change the
>>> documentation.
>> Forgot to attach rebased patches.
>
> Another rebase needed so quickly? :) Thanks for the assist!
>
> Regards,
> Mark
I still have the question as I raised previously. Let’s use 0001 as an example. 0001 removes function 1573.
Say, an existing view depends on 1573:
```
evantest=# CREATE VIEW v_ruledef AS
evantest-# SELECT pg_get_ruledef(oid) AS ruledef
evantest-# FROM pg_rewrite
evantest-# WHERE rulename = '_RETURN';
CREATE VIEW
```
The view is stored as:
```
_RETURN | v_ruledef | ({QUERY :commandType 1 :querySource 0 :canSetTag true :utilityStmt <> :resultRelation 0 :forPortionOf <> :hasAggs false :hasWindowFuncs false :hasTargetSRFs false :hasSubLinks false :hasDistinctOn false :hasRecursive false :hasModifyingCTE false :hasForUpdate false :hasRowSecurity false :hasGroupRTE false :isReturn false :cteList <> :rtable ({RANGETBLENTRY :alias <> :eref {ALIAS :aliasname pg_rewrite :colnames ("oid" "rulename" "ev_class" "ev_type" "ev_enabled" "is_instead" "ev_qual" "ev_action")} :rtekind 0 :relid 2618 :inh true :relkind r :rellockmode 1 :perminfoindex 1 :tablesample <> :lateral false :inFromCl true :securityQuals <>}) :rteperminfos ({RTEPERMISSIONINFO :relid 2618 :inh true :requiredPerms 2 :checkAsUser 0 :selectedCols (b 8 9) :insertedCols (b) :updatedCols (b)}) :jointree {FROMEXPR :fromlist ({RANGETBLREF :rtindex 1}) :quals {OPEXPR :opno 93 :opfuncid 62 :opresulttype 16 :opretset false :opcollid 0 :inputcollid 950 :args ({VAR :varno 1 :varattno 2 :vartype 19 :vartypmod -1 :varcollid 950 :varnullingrels (b) :varlevelsup 0 :varreturningtype 0 :varnosyn 1 :varattnosyn 2 :location -1} {CONST :consttype 19 :consttypmod -1 :constcollid 950 :constlen 64 :constbyval false :constisnull false :location -1 :constvalue 64 [ 95 82 69 84 85 82 78 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 ]}) :location -1}} :mergeActionList <> :mergeTargetRelation 0 :mergeJoinCondition <> :targetList ({TARGETENTRY :expr {FUNCEXPR :funcid 1573 :funcresulttype 25 :funcretset false :funcvariadic false :funcformat 0 :funccollid 100 :inputcollid 0 :args ({VAR :varno 1 :varattno 1 :vartype 26 :vartypmod -1 :varcollid 0 :varnullingrels (b) :varlevelsup 0 :varreturningtype 0 :varnosyn 1 :varattnosyn 1 :location -1}) :location -1} :resno 1 :resname ruledef :ressortgroupref 0 :resorigtbl 0 :resorigcol 0 :resjunk false}) :override 0 :onConflict <> :returningOldAlias <> :returningNewAlias <> :returningList <> :groupClause <> :groupDistinct false :groupByAll false :groupingSets <> :havingQual <> :windowClause <> :distinctClause <> :sortClause <> :limitOffset <> :limitCount <> :limitOption 0 :rowMarks <> :setOperations <> :constraintDeps <> :withCheckOptions <> :stmt_location -1 :stmt_len -1})
```
We can clearly see ":expr {FUNCEXPR :funcid 1573 “.
With this patch, will that view break? How would users find all such broken views? Maybe PostgreSQL already has some recommended way to handle this kind of situation that I am not aware of?
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-08 01:46 David G. Johnston <[email protected]>
parent: Chao Li <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: David G. Johnston @ 2026-04-08 01:46 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Mark Wong <[email protected]>; Andreas Karlsson <[email protected]>; [email protected] <[email protected]>
On Tuesday, April 7, 2026, Chao Li <[email protected]> wrote:
>
> We can clearly see ":expr {FUNCEXPR :funcid 1573 “.
>
> With this patch, will that view break? How would users find all such
> broken views? Maybe PostgreSQL already has some recommended way to handle
> this kind of situation that I am not aware of?
>
pg_dump resolves oid=1573 and produces a textual SQL representation, which
is then executed during pg_restore. This happens manually, and also
automatically by pg_upgrade. Since the text form hasn’t changed the view
is still valid in v19. You would see the new oid if inspecting the rule
after the upgrade.
So yes, the public serialization format being SQL and thus mandatory new
object creation during upgrade is the way PostgreSQL handles implementation
detail isolation.
David J.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-08 02:09 Chao Li <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Chao Li @ 2026-04-08 02:09 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Mark Wong <[email protected]>; Andreas Karlsson <[email protected]>; [email protected] <[email protected]>
> On Apr 8, 2026, at 09:46, David G. Johnston <[email protected]> wrote:
>
> On Tuesday, April 7, 2026, Chao Li <[email protected]> wrote:
>
> We can clearly see ":expr {FUNCEXPR :funcid 1573 “.
>
> With this patch, will that view break? How would users find all such broken views? Maybe PostgreSQL already has some recommended way to handle this kind of situation that I am not aware of?
>
> pg_dump resolves oid=1573 and produces a textual SQL representation, which is then executed during pg_restore. This happens manually, and also automatically by pg_upgrade. Since the text form hasn’t changed the view is still valid in v19. You would see the new oid if inspecting the rule after the upgrade.
>
> So yes, the public serialization format being SQL and thus mandatory new object creation during upgrade is the way PostgreSQL handles implementation detail isolation.
>
> David J.
>
Hi David, thanks for the explanation, I really didn’t know that.
Then, the patch looks good to me. GET_PRETTY_FLAGS(false) returns PRETTYFLAG_INDENT, so no behavior change either.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-08 18:10 Mark Wong <[email protected]>
parent: Andreas Karlsson <[email protected]>
0 siblings, 2 replies; 18+ messages in thread
From: Mark Wong @ 2026-04-08 18:10 UTC (permalink / raw)
To: Andreas Karlsson <[email protected]>; +Cc: [email protected]
On Wed, Apr 08, 2026 at 01:51:30AM +0200, Andreas Karlsson wrote:
> On 4/8/26 1:42 AM, Mark Wong wrote:
> > I don't see any matches when I grep for rule_oid or pretty_bool...
>
> Sorry for the noise, I was looking at an old version of the docs. The
> parameters do indeed match the docs. But now that I looked at the
> patches again I found a real issue which I think should be fixed.
No worries.
> I think the following:
>
> "select statement of a view with pretty-print option"
>
> should likely just be:
>
> "select statement of a view"
Yeah, I agree with that considering that other functions like the recent
get_*_ddl functions have options like that aren't detailed in
pg_proc.dat.
What I pondered over a little bit was whether to flip-flop and remove
the original _ext definitions instead and modify their respective
counterparts that I originally removed. I opted to continue editing what
I started because of the comment that reads "System-view support
functions with pretty-print option", but I don't have any strong
opinions either way.
I've attached v7 with the more succinct descriptions.
Regards,
Mark
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-04-08 19:18 Andreas Karlsson <[email protected]>
parent: Mark Wong <[email protected]>
1 sibling, 0 replies; 18+ messages in thread
From: Andreas Karlsson @ 2026-04-08 19:18 UTC (permalink / raw)
To: Mark Wong <[email protected]>; +Cc: [email protected]
On 4/8/26 8:10 PM, Mark Wong wrote:
> What I pondered over a little bit was whether to flip-flop and remove
> the original _ext definitions instead and modify their respective
> counterparts that I originally removed. I opted to continue editing what
> I started because of the comment that reads "System-view support
> functions with pretty-print option", but I don't have any strong
> opinions either way.
>
> I've attached v7 with the more succinct descriptions.
I would be fine with either too. The comment feels a bit less useful now
so it too could either stay or go. I have no strong feelings about it.
Setting this as ready for committer since I think the patch is fine
as-is and an improvement over the current no matter what is done about
the comment or which oids we decide to use.
Andreas
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-06-05 20:14 Mark Wong <[email protected]>
parent: Mark Wong <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Mark Wong @ 2026-06-05 20:14 UTC (permalink / raw)
To: [email protected]
I noticed a rebase was needed so I've attached v8.
Regards,
Mark
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-06-05 21:04 Tristan Partin <[email protected]>
parent: Mark Wong <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2026-06-05 21:04 UTC (permalink / raw)
To: Mark Wong <[email protected]>; +Cc: [email protected]
> Patch 1
>
> - proargtypes => 'oid bool', prosrc => 'pg_get_ruledef_ext' },
> + proargtypes => 'oid bool', proargnames => '{rule,pretty}',
> + proargdefaults => '{false}',prosrc => 'pg_get_ruledef' },
There is a space missing before prosrc in the added line.
Other than that one comment, this looks good to me.
I am a little confused how proargdefaults works given the function takes
two arguments, but you only defined one default, so how does the parser
know that the false default is in reference to "pretty". I do see there
is other precedent for this pattern, so it is correct.
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-06-11 14:47 Mark Wong <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Mark Wong @ 2026-06-11 14:47 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; +Cc: [email protected]
On Fri, Jun 05, 2026 at 09:04:34PM +0000, Tristan Partin wrote:
> > Patch 1
> >
> > - proargtypes => 'oid bool', prosrc => 'pg_get_ruledef_ext' },
> > + proargtypes => 'oid bool', proargnames => '{rule,pretty}',
> > + proargdefaults => '{false}',prosrc => 'pg_get_ruledef' },
>
> There is a space missing before prosrc in the added line.
>
> Other than that one comment, this looks good to me.
Oops, v8.1 attached to correct that white space.
> I am a little confused how proargdefaults works given the function takes
> two arguments, but you only defined one default, so how does the parser
> know that the false default is in reference to "pretty". I do see there
> is other precedent for this pattern, so it is correct.
I believe that's handled in InsertOneProargdefaultsValue() in
src/backend/bootstrap/bootstrap.c, right below the comment:
/* The values should correspond to the last N argtypes */
Regards,
Mark
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: updates for handling optional argument in system functions
@ 2026-06-18 16:50 Tristan Partin <[email protected]>
parent: Mark Wong <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Tristan Partin @ 2026-06-18 16:50 UTC (permalink / raw)
To: Mark Wong <[email protected]>; +Cc: [email protected]
On Thu Jun 11, 2026 at 2:47 PM UTC, Mark Wong wrote:
> On Fri, Jun 05, 2026 at 09:04:34PM +0000, Tristan Partin wrote:
>> > Patch 1
>> >
>> > - proargtypes => 'oid bool', prosrc => 'pg_get_ruledef_ext' },
>> > + proargtypes => 'oid bool', proargnames => '{rule,pretty}',
>> > + proargdefaults => '{false}',prosrc => 'pg_get_ruledef' },
>>
>> There is a space missing before prosrc in the added line.
>>
>> Other than that one comment, this looks good to me.
>
> Oops, v8.1 attached to correct that white space.
Patches look great Mark!
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2026-06-18 16:50 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-12-09 23:28 updates for handling optional argument in system functions Mark Wong <[email protected]>
2025-12-10 09:33 ` Daniel Gustafsson <[email protected]>
2026-04-02 18:36 ` Mark Wong <[email protected]>
2026-04-07 22:26 ` Andreas Karlsson <[email protected]>
2026-04-07 23:28 ` Andreas Karlsson <[email protected]>
2026-04-07 23:44 ` Mark Wong <[email protected]>
2026-04-08 01:33 ` Chao Li <[email protected]>
2026-04-08 01:46 ` David G. Johnston <[email protected]>
2026-04-08 02:09 ` Chao Li <[email protected]>
2026-04-07 23:42 ` Mark Wong <[email protected]>
2026-04-07 23:51 ` Andreas Karlsson <[email protected]>
2026-04-08 18:10 ` Mark Wong <[email protected]>
2026-04-08 19:18 ` Andreas Karlsson <[email protected]>
2026-06-05 20:14 ` Mark Wong <[email protected]>
2026-06-05 21:04 ` Tristan Partin <[email protected]>
2026-06-11 14:47 ` Mark Wong <[email protected]>
2026-06-18 16:50 ` Tristan Partin <[email protected]>
2026-03-12 03:28 Re: updates for handling optional argument in system functions Mark Wong <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox