mention unused_oids script in pg_proc.dat

Started by Florents Tselai11 months ago10 messageshackers
Jump to latest
#1Florents Tselai
florents.tselai@gmail.com

Attachments:

v1-0001-Add-pg_proc.dat-comment-mentioning-the-unused_oid.patchapplication/octet-stream; name=v1-0001-Add-pg_proc.dat-comment-mentioning-the-unused_oid.patchDownload+3-1
#2Michael Paquier
michael@paquier.xyz
In reply to: Florents Tselai (#1)
Re: mention unused_oids script in pg_proc.dat

On Fri, May 23, 2025 at 07:03:31PM +0300, Florents Tselai wrote:

-

For the sake of the archives: this has been mentioned on Discord.

+++ b/src/include/catalog/pg_proc.dat
+# To find unused OIDs see src/include/catalog/unused_oids.
+# If multiple related OIDs are added, it is suggested they're contiguous.

This stuff is already documented in bki.sgml. Adding an extra
location to hint about the script may be useful to have, but
pg_proc.dat is not a good choice as it is one catalog among many
others. One idea: we could add a README or a README.oid in
src/include/catalog/ telling more about unused_oids, renumber_oids and
duplicate_oids, still it would be a duplication of what the
documentation already informs about. Another idea could be just to
provide a link redirecting to the "current" or "in-development" docs,
stored in a README in src/include/catalog/.

The issue seems to be the discoverability of these scripts for new
developers. If others have ideas and/or opinions, feel free.
--
Michael

#3Florents Tselai
florents.tselai@gmail.com
In reply to: Michael Paquier (#2)
Re: mention unused_oids script in pg_proc.dat

This stuff is already documented in bki.sgml. Adding an extra
location to hint about the script may be useful to have, but
pg_proc.dat is not a good choice as it is one catalog among many
others. One idea: we could add a README or a README.oid in
src/include/catalog/ telling more about unused_oids, renumber_oids and
duplicate_oids, still it would be a duplication of what the
documentation already informs about. Another idea could be just to
provide a link redirecting to the "current" or "in-development" docs,
stored in a README in src/include/catalog/.

I aggree with having more READMEs around the src tree.
It’s true that a lot of doc content found in VII. Internals is dev-oriented,
but imho it should be closer to the src (and more succinct/less verbose too).

Looking at some of the text in 67.2.2. OID Assignment for example,
a few things look outdated wrt choosing OIDs.

One can look through the history of similar commits to see what
needs to be changed; So it’s not THAT bad.
It’s just that script runs are not visible in git.

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Florents Tselai (#3)
Re: mention unused_oids script in pg_proc.dat

On 2025-May-24, Florents Tselai wrote:

I aggree with having more READMEs around the src tree.
It’s true that a lot of doc content found in VII. Internals is dev-oriented,
but imho it should be closer to the src (and more succinct/less verbose too).

Maybe these READMEs can simply contain little more than links to the
built HTML docs, to avoid having multiple sources of info.

Looking at some of the text in 67.2.2. OID Assignment for example,
a few things look outdated wrt choosing OIDs.

One can look through the history of similar commits to see what
needs to be changed; So it’s not THAT bad.

Sure, outdated docs is something that happens. Patches welcome.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Cómo ponemos nuestros dedos en la arcilla del otro. Eso es la amistad; jugar
al alfarero y ver qué formas se pueden sacar del otro" (C. Halloway en
La Feria de las Tinieblas, R. Bradbury)

#5Florents Tselai
florents.tselai@gmail.com
In reply to: Alvaro Herrera (#4)
Re: mention unused_oids script in pg_proc.dat

On 24 May 2025, at 12:24 PM, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-May-24, Florents Tselai wrote:

I aggree with having more READMEs around the src tree.
It’s true that a lot of doc content found in VII. Internals is dev-oriented,
but imho it should be closer to the src (and more succinct/less verbose too).

Maybe these READMEs can simply contain little more than links to the
built HTML docs, to avoid having multiple sources of info.

Looking at some of the text in 67.2.2. OID Assignment for example,
a few things look outdated wrt choosing OIDs.

One can look through the history of similar commits to see what
needs to be changed; So it’s not THAT bad.

Sure, outdated docs is something that happens. Patches welcome.

Something like this ?

Attachments:

v1-catalog-readme.patchapplication/octet-stream; name=v1-catalog-readme.patch; x-unix-mode=0644Download+11-1
#6Peter Eisentraut
peter_e@gmx.net
In reply to: Florents Tselai (#5)
Re: mention unused_oids script in pg_proc.dat

On 24.05.25 12:34, Florents Tselai wrote:

On 24 May 2025, at 12:24 PM, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-May-24, Florents Tselai wrote:

I aggree with having more READMEs around the src tree.
It’s true that a lot of doc content found in VII. Internals is dev-oriented,
but imho it should be closer to the src (and more succinct/less verbose too).

Maybe these READMEs can simply contain little more than links to the
built HTML docs, to avoid having multiple sources of info.

Looking at some of the text in 67.2.2. OID Assignment for example,
a few things look outdated wrt choosing OIDs.

One can look through the history of similar commits to see what
needs to be changed; So it’s not THAT bad.

Sure, outdated docs is something that happens. Patches welcome.

Something like this ?

diff --git a/src/include/catalog/README b/src/include/catalog/README
new file mode 100644
index 00000000000..5bd81c6d86c
--- /dev/null
+++ b/src/include/catalog/README
@@ -0,0 +1,11 @@
+# Catalog
+
+For more details see https://www.postgresql.org/docs/current/bki.html

I can see that having a README in src/include/catalog/ would be useful.
That directory contains some "unusual" files, and some hints about them
would be good. Before commit 372728b0d49, we used to have a README in
src/backend/catalog/README, but then this was moved to the SGML
documentation. Maybe you can read up there why this was chosen. But I
think src/include/catalog/ would be a better location in any case.

Maybe for now just a link like you show would be ok. (But it should
probably be s/current/devel/.)

+
+Below are some checklists for common scenarios.
+
+## Adding a New Built-in SQL Command
+
+1. Implement the function in C and place it in the appropriate 

directory under `src` (typically in `src/utils/adt`)

+2. Each function should have a unique integer OID. Run the script

`src/include/catalog/unused_oids` to find available ranges.

+3. Add the entry to `pg_proc.dat` following the surrounding conventions.

Presumably, if you're developing this kind of thing, you are already
reading some other part of the documentation, maybe xfunc.sgml? Do we
need to add more information there, or some links?

#7Florents Tselai
florents.tselai@gmail.com
In reply to: Peter Eisentraut (#6)
Re: mention unused_oids script in pg_proc.dat

On Tue, Jul 1, 2025 at 12:25 PM Peter Eisentraut <peter@eisentraut.org>
wrote:

On 24.05.25 12:34, Florents Tselai wrote:

On 24 May 2025, at 12:24 PM, Álvaro Herrera <alvherre@kurilemu.de>

wrote:

On 2025-May-24, Florents Tselai wrote:

I aggree with having more READMEs around the src tree.
It’s true that a lot of doc content found in VII. Internals is

dev-oriented,

but imho it should be closer to the src (and more succinct/less

verbose too).

Maybe these READMEs can simply contain little more than links to the
built HTML docs, to avoid having multiple sources of info.

Looking at some of the text in 67.2.2. OID Assignment for example,
a few things look outdated wrt choosing OIDs.

One can look through the history of similar commits to see what
needs to be changed; So it’s not THAT bad.

Sure, outdated docs is something that happens. Patches welcome.

Something like this ?

diff --git a/src/include/catalog/README b/src/include/catalog/README
new file mode 100644
index 00000000000..5bd81c6d86c
--- /dev/null
+++ b/src/include/catalog/README
@@ -0,0 +1,11 @@
+# Catalog
+
+For more details see https://www.postgresql.org/docs/current/bki.html

I can see that having a README in src/include/catalog/ would be useful.
That directory contains some "unusual" files, and some hints about them
would be good. Before commit 372728b0d49, we used to have a README in
src/backend/catalog/README, but then this was moved to the SGML
documentation. Maybe you can read up there why this was chosen. But I
think src/include/catalog/ would be a better location in any case.

In practice people go to the git history to see the files touched by
similar commits,
and devise a to-do-list for their case.
so I think it helps them a lot to see some high-level READMEs waiting for
them there.

In this particular case, unused_oids isn't included in any commits so you
can't be aware of its existence even

Maybe for now just a link like you show would be ok. (But it should
probably be s/current/devel/.)

Done

+
+Below are some checklists for common scenarios.
+
+## Adding a New Built-in SQL Command
+
+1. Implement the function in C and place it in the appropriate

directory under `src` (typically in `src/utils/adt`)

+2. Each function should have a unique integer OID. Run the script

`src/include/catalog/unused_oids` to find available ranges.

+3. Add the entry to `pg_proc.dat` following the surrounding

conventions.

Presumably, if you're developing this kind of thing, you are already
reading some other part of the documentation, maybe xfunc.sgml? Do we
need to add more information there, or some links?

I agree. Added a link reference to that xfunc-c

Some basic stuff that was missing in xfunc, I've already added in 86e4efc .
For now I think it's good enough.

Attachments:

v2-0001-Add-catalog-README.patchapplication/octet-stream; name=v2-0001-Add-catalog-README.patchDownload+14-1
#8Michael Paquier
michael@paquier.xyz
In reply to: Florents Tselai (#7)
Re: mention unused_oids script in pg_proc.dat

On Tue, Jul 08, 2025 at 05:40:39PM +0300, Florents Tselai wrote:

On Tue, Jul 1, 2025 at 12:25 PM Peter Eisentraut <peter@eisentraut.org> wrote:

+
+Below are some checklists for common scenarios.
+
+## Adding a New Built-in SQL Command
+
+1. Implement the function in C and place it in the appropriate

directory under `src` (typically in `src/utils/adt`)

+2. Each function should have a unique integer OID. Run the script

`src/include/catalog/unused_oids` to find available ranges.

+3. Add the entry to `pg_proc.dat` following the surrounding

conventions.

Presumably, if you're developing this kind of thing, you are already
reading some other part of the documentation, maybe xfunc.sgml? Do we
need to add more information there, or some links?

I agree. Added a link reference to that xfunc-c

I understand Peter's comment that we need something more general than
what your v2-0001 is proposing, the point being that modifying
pg_proc.dat may apply to some concepts, but it's far from being all of
them. If you need to add a new in-core type, or something else to a
different catalog, then you need to be aware of a completely different
.dat file. This set of instructions read as being a bit misleading,
IMO.
--
Michael

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#8)
Re: mention unused_oids script in pg_proc.dat

On 09.07.25 01:28, Michael Paquier wrote:

On Tue, Jul 08, 2025 at 05:40:39PM +0300, Florents Tselai wrote:

On Tue, Jul 1, 2025 at 12:25 PM Peter Eisentraut <peter@eisentraut.org> wrote:

+
+Below are some checklists for common scenarios.
+
+## Adding a New Built-in SQL Command
+
+1. Implement the function in C and place it in the appropriate

directory under `src` (typically in `src/utils/adt`)

+2. Each function should have a unique integer OID. Run the script

`src/include/catalog/unused_oids` to find available ranges.

+3. Add the entry to `pg_proc.dat` following the surrounding

conventions.

Presumably, if you're developing this kind of thing, you are already
reading some other part of the documentation, maybe xfunc.sgml? Do we
need to add more information there, or some links?

I agree. Added a link reference to that xfunc-c

I understand Peter's comment that we need something more general than
what your v2-0001 is proposing, the point being that modifying
pg_proc.dat may apply to some concepts, but it's far from being all of
them. If you need to add a new in-core type, or something else to a
different catalog, then you need to be aware of a completely different
.dat file. This set of instructions read as being a bit misleading,
IMO.

I have committed the part that adds a link to the bki documentation.

I think the other parts, like how to add a new built-in function, didn't
have consensus. Maybe a wiki page or a blog post could be good forums
for that.

#10Florents Tselai
florents.tselai@gmail.com
In reply to: Peter Eisentraut (#9)
Re: mention unused_oids script in pg_proc.dat

I have committed the part that adds a link to the bki documentation.

Thnx.
That's good enough for me for now.