Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Simplify creation of built-in functions with default arguments.

  1. updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2025-12-09T23:28:59Z

    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 <markwkm@gmail.com>
EDB https://enterprisedb.com

  2. Re: updates for handling optional argument in system functions

    Daniel Gustafsson <daniel@yesql.se> — 2025-12-10T09:33:05Z

    > On 10 Dec 2025, at 00:28, Mark Wong <markwkm@gmail.com> 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

  3. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-01-20T18:15:13Z

    Hi everyone,

On Tue, Dec 09, 2025 at 03:28:59PM -0800, Mark Wong wrote:
> 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?

I fixed an error caught by the address sanitizer in CI [1] and am uploading a
new patchset.  The only change is to 2 lines in
v2-0005-Handle-pg_get_expr-default-args-in-system_functio.patch to update a
call to pg_get_expr with the correct number of arguments in tablecmds.c.

Regards,
Mark

[1] https://cirrus-ci.com/task/6109065824174080
--
Mark Wong <markwkm@gmail.com>
EDB https://enterprisedb.com

  4. Re: updates for handling optional argument in system functions

    Chao Li <li.evan.chao@gmail.com> — 2026-01-21T08:45:35Z

    
> On Jan 21, 2026, at 02:15, Mark Wong <markwkm@gmail.com> wrote:
> 
> Hi everyone,
> 
> On Tue, Dec 09, 2025 at 03:28:59PM -0800, Mark Wong wrote:
>> 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?
> 
> I fixed an error caught by the address sanitizer in CI [1] and am uploading a
> new patchset.  The only change is to 2 lines in
> v2-0005-Handle-pg_get_expr-default-args-in-system_functio.patch to update a
> call to pg_get_expr with the correct number of arguments in tablecmds.c.
> 
> Regards,
> Mark
> 
> [1] https://cirrus-ci.com/task/6109065824174080
> --
> Mark Wong <markwkm@gmail.com>
> EDB https://enterprisedb.com
> <v2-0001-Handle-pg_get_ruledef-default-args-in-system_func.patch><v2-0002-Handle-pg_get_viewdef-default-args-in-system_func.patch><v2-0003-Handle-pg_get_indexdef-default-args-in-system_fun.patch><v2-0004-Handle-pg_get_constraintdef-default-args-in-syste.patch><v2-0005-Handle-pg_get_expr-default-args-in-system_functio.patch><v2-0006-Handle-pg_get_triggerdef-default-args-in-system_f.patch>

Hi Mark,

Thanks for the patch. After reviewing it, I have mixed feelings. From one side, it removes some redundant code, which is good. In the other side, I doubt if we should delete proc entries from pg_proc.c? Say, there is a view that uses a proc to be deleted, the proc OID is stored with the view, then after an upgrade, the view would be broken. From this perspective, should we retain the old proc entries and only point them to the new functions?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  5. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-01-30T00:15:18Z

    On Wed, Jan 21, 2026 at 04:45:35PM +0800, Chao Li wrote:
> 
> 
> > On Jan 21, 2026, at 02:15, Mark Wong <markwkm@gmail.com> wrote:
> > 
> > Hi everyone,
> > 
> > On Tue, Dec 09, 2025 at 03:28:59PM -0800, Mark Wong wrote:
> >> 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?
> > 
> > I fixed an error caught by the address sanitizer in CI [1] and am uploading a
> > new patchset.  The only change is to 2 lines in
> > v2-0005-Handle-pg_get_expr-default-args-in-system_functio.patch to update a
> > call to pg_get_expr with the correct number of arguments in tablecmds.c.
> > 
> > Regards,
> > Mark
> > 
> > [1] https://cirrus-ci.com/task/6109065824174080
> 
> Hi Mark,
> 
> Thanks for the patch. After reviewing it, I have mixed feelings. From one side, it removes some redundant code, which is good. In the other side, I doubt if we should delete proc entries from pg_proc.c? Say, there is a view that uses a proc to be deleted, the proc OID is stored with the view, then after an upgrade, the view would be broken. From this perspective, should we retain the old proc entries and only point them to the new functions?

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.

Maybe that can also be used towards finding that particular solution...

Regards,
Mark
-- 
Mark Wong <markwkm@gmail.com>
EDB https://enterprisedb.com

  6. Re: updates for handling optional argument in system functions

    Andreas Karlsson <andreas@proxel.se> — 2026-02-18T21:56:56Z

    On 1/30/26 1:15 AM, Mark Wong wrote:
>> Thanks for the patch. After reviewing it, I have mixed feelings. From one side, it removes some redundant code, which is good. In the other side, I doubt if we should delete proc entries from pg_proc.c? Say, there is a view that uses a proc to be deleted, the proc OID is stored with the view, then after an upgrade, the view would be broken. From this perspective, should we retain the old proc entries and only point them to the new functions?
> 
> 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.
> 
> Maybe that can also be used towards finding that particular solution...
I am not sure what can be done, breaking people's databases on 
pg_upgrade is certainly not nice and detecting that function oid has 
been used anywhere in a database sounds painful, especially since there 
are no references to system oids in pg_depend, right?

That said this patch should be updated to use the new support for 
default values in BKI files.[1]

https://git.postgresql.org/cgit/postgresql.git/commit/?id=759b03b24ce96f0ba6d734b570d1a6f4a0fb1177

Andreas

  7. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-02-26T19:43:30Z

    On Wed, Feb 18, 2026 at 10:56:56PM +0100, Andreas Karlsson wrote:
> On 1/30/26 1:15 AM, Mark Wong wrote:
> > > Thanks for the patch. After reviewing it, I have mixed feelings. From one side, it removes some redundant code, which is good. In the other side, I doubt if we should delete proc entries from pg_proc.c? Say, there is a view that uses a proc to be deleted, the proc OID is stored with the view, then after an upgrade, the view would be broken. From this perspective, should we retain the old proc entries and only point them to the new functions?
> > 
> > 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.
> > 
> > Maybe that can also be used towards finding that particular solution...
> I am not sure what can be done, breaking people's databases on pg_upgrade is
> certainly not nice and detecting that function oid has been used anywhere in
> a database sounds painful, especially since there are no references to
> system oids in pg_depend, right?
> 
> That said this patch should be updated to use the new support for default
> values in BKI files.[1]
> 
> https://git.postgresql.org/cgit/postgresql.git/commit/?id=759b03b24ce96f0ba6d734b570d1a6f4a0fb1177

I have attached a new set of patches.  v4 is now using the new support for
default values.

Summary of additional changes:

* I've removed the retired OID tracking, but can certainly add that back if we
  decide it will be useful
* Caught a bug where I wasn't using BoolGetDatum() with DirectFunctionCall3
  with pg_get_expr()


Regards,
Mark
-- 
Mark Wong <markwkm@gmail.com>
EDB https://enterprisedb.com

  8. Re: updates for handling optional argument in system functions

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-03-03T16:23:58Z

    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?

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Syntax error: function hell() needs an argument.
Please choose what hell you want to involve.

  9. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-03-12T03:28:14Z

    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 <markwkm@gmail.com>
EDB https://enterprisedb.com

  10. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-04-02T18:36:11Z

    Hi everyone, 

I've attached v5, simply a needed rebase due to some other churn in
pg_proc.dat.

Regards,
Mark

  11. Re: updates for handling optional argument in system functions

    Andreas Karlsson <andreas@proxel.se> — 2026-04-07T22:26:05Z

    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

  12. Re: updates for handling optional argument in system functions

    Andreas Karlsson <andreas@proxel.se> — 2026-04-07T23:28:29Z

    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

  13. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-04-07T23:42:23Z

    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

  14. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-04-07T23:44:05Z

    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

  15. Re: updates for handling optional argument in system functions

    Andreas Karlsson <andreas@proxel.se> — 2026-04-07T23:51:30Z

    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

  16. Re: updates for handling optional argument in system functions

    Chao Li <li.evan.chao@gmail.com> — 2026-04-08T01:33:56Z

    
> On Apr 8, 2026, at 07:44, Mark Wong <markwkm@gmail.com> 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/

  17. Re: updates for handling optional argument in system functions

    David G. Johnston <david.g.johnston@gmail.com> — 2026-04-08T01:46:17Z

    On Tuesday, April 7, 2026, Chao Li <li.evan.chao@gmail.com> 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.

  18. Re: updates for handling optional argument in system functions

    Chao Li <li.evan.chao@gmail.com> — 2026-04-08T02:09:51Z

    
> On Apr 8, 2026, at 09:46, David G. Johnston <david.g.johnston@gmail.com> wrote:
> 
> On Tuesday, April 7, 2026, Chao Li <li.evan.chao@gmail.com> 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/

  19. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-04-08T18:10:55Z

    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

  20. Re: updates for handling optional argument in system functions

    Andreas Karlsson <andreas@proxel.se> — 2026-04-08T19:18:34Z

    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

  21. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-06-05T20:14:12Z

    I noticed a rebase was needed so I've attached v8.

Regards,
Mark

  22. Re: updates for handling optional argument in system functions

    Tristan Partin <tristan@partin.io> — 2026-06-05T21:04:34Z

    > 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)

  23. Re: updates for handling optional argument in system functions

    Mark Wong <markwkm@gmail.com> — 2026-06-11T14:47:37Z

    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

  24. Re: updates for handling optional argument in system functions

    Tristan Partin <tristan@partin.io> — 2026-06-18T16:50:13Z

    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)

  25. Re: updates for handling optional argument in system functions

    Fujii Masao <masao.fujii@gmail.com> — 2026-07-03T13:59:19Z

    On Fri, Jun 19, 2026 at 1:50 AM Tristan Partin <tristan@partin.io> wrote:
> > Oops, v8.1 attached to correct that white space.
>
> Patches look great Mark!

Some patches no longer apply cleanly on HEAD, so they'll need to be rebased.

I have one question, though.

Suppose a user executes GRANT or REVOKE on pg_get_ruledef(oid), then
creates a dump with pg_dump. If that dump is restored into a newer
server where these patches have been applied, the restore would fail
because pg_get_ruledef(oid) no longer exists.

Is that acceptable (this means that users need to handle the restore
failure), or should pg_dump handle this case specially?

I suspect we've had similar situations before when system functions
changed between major versions, and I don't recall adding special
handling to pg_dump... Thought?

Regards,

-- 
Fujii Masao

  26. Re: updates for handling optional argument in system functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-03T14:15:02Z

    Fujii Masao <masao.fujii@gmail.com> writes:
> I have one question, though.

> Suppose a user executes GRANT or REVOKE on pg_get_ruledef(oid), then
> creates a dump with pg_dump. If that dump is restored into a newer
> server where these patches have been applied, the restore would fail
> because pg_get_ruledef(oid) no longer exists.

> Is that acceptable (this means that users need to handle the restore
> failure), or should pg_dump handle this case specially?

I wouldn't blink an eye at that.  There's no obvious use-case for
someone to mess with the permissions on that function.  Even
if there were, I don't think it'd be reasonable for pg_dump to
try to patch it up.  (I'm not sure that pg_dump would dump such
grants in the first place.)

			regards, tom lane

  27. Re: updates for handling optional argument in system functions

    Fujii Masao <masao.fujii@gmail.com> — 2026-07-03T15:01:57Z

    On Fri, Jul 3, 2026 at 11:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fujii Masao <masao.fujii@gmail.com> writes:
> > I have one question, though.
>
> > Suppose a user executes GRANT or REVOKE on pg_get_ruledef(oid), then
> > creates a dump with pg_dump. If that dump is restored into a newer
> > server where these patches have been applied, the restore would fail
> > because pg_get_ruledef(oid) no longer exists.
>
> > Is that acceptable (this means that users need to handle the restore
> > failure), or should pg_dump handle this case specially?
>
> I wouldn't blink an eye at that.  There's no obvious use-case for
> someone to mess with the permissions on that function.  Even
> if there were, I don't think it'd be reasonable for pg_dump to
> try to patch it up.  (I'm not sure that pg_dump would dump such
> grants in the first place.)

pg_dump seems to dump such grants. For example:

    $ createuser foo
    $ psql -c "REVOKE ALL ON FUNCTION pg_get_ruledef(oid) FROM public"
    REVOKE
    $ psql -c "GRANT ALL ON FUNCTION pg_get_ruledef(oid) TO foo"
    GRANT
    $ pg_dump | grep pg_get_ruledef
    -- Name: FUNCTION pg_get_ruledef(oid); Type: ACL; Schema:
pg_catalog; Owner: postgres
    REVOKE ALL ON FUNCTION pg_catalog.pg_get_ruledef(oid) FROM PUBLIC;
    GRANT ALL ON FUNCTION pg_catalog.pg_get_ruledef(oid) TO foo;

My concern was simply that restoring such a dump into a newer server
could fail because the single-argument function like pg_get_ruledef(oid)
no longer exists. This could affect not only GRANT/REVOKE, but also
commands such as ALTER FUNCTION and COMMENT, etc.
So I just asked that question.

If we agree that these are fairly unlikely use cases, so no special
handling for such restore failures is necessary, I'm ok with that.

Regards,

-- 
Fujii Masao