regdatabase

Started by Nathan Bossart10 months ago23 messages
Jump to latest
#1Nathan Bossart
nathandbossart@gmail.com

Every once in a while, I find myself wanting to use regdatabase for
something like current_database()::regdatabase, and I'm always surprised
when I inevitably rediscover that it doesn't exist. I only found one
reference to the idea in the archives [0]/messages/by-id/20191109220939.jz55zcc33d3g7h7b@alap3.anarazel.de. So, I have two questions:

* Is there an easier way to get the current database's (or any database's)
OID that I am overlooking (besides "SELECT oid FROM pg_database...")?

* Would anyone object if I put together some patches to add regdatabase?

[0]: /messages/by-id/20191109220939.jz55zcc33d3g7h7b@alap3.anarazel.de

--
nathan

#2Tatsuo Ishii
ishii@postgresql.org
In reply to: Nathan Bossart (#1)
Re: regdatabase

Every once in a while, I find myself wanting to use regdatabase for
something like current_database()::regdatabase, and I'm always surprised
when I inevitably rediscover that it doesn't exist. I only found one
reference to the idea in the archives [0]. So, I have two questions:

* Is there an easier way to get the current database's (or any database's)
OID that I am overlooking (besides "SELECT oid FROM pg_database...")?

I always do "SELECT oid FROM pg_database...". I have no idea other way.

* Would anyone object if I put together some patches to add regdatabase?

I think regdatabase is a good idea.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

#3Michael Paquier
michael@paquier.xyz
In reply to: Tatsuo Ishii (#2)
Re: regdatabase

On Wed, May 07, 2025 at 07:28:02AM +0900, Tatsuo Ishii wrote:

I think regdatabase is a good idea.

I've also found your reference from the lists of 2019 to be the only
one referring to a regdatabase.

If it means that I will type less by not having to do joins with
pg_database, count me in.

The last time I recall we've discussed about a reg shortcut was
regcollation, which was mentioned as useful for dumps. Even if we've
finished by not using it in the dump, I tend to find these shortcuts
always useful.

For regdatabase, there would be at least two simplications related to
the dump of subscriptions, where we could switch the queries to the
new grammar for backend versions able to support the new grammar,
meaning that we could remove entirely these parts in 11~12 years once
the versions that do not support the new reg shortcut would be out of
scope. :D
--
Michael

#4Ian Lawrence Barwick
barwick@gmail.com
In reply to: Nathan Bossart (#1)
Re: regdatabase

2025年5月7日(水) 4:29 Nathan Bossart <nathandbossart@gmail.com>:

Every once in a while, I find myself wanting to use regdatabase for
something like current_database()::regdatabase, and I'm always surprised
when I inevitably rediscover that it doesn't exist. I only found one
reference to the idea in the archives [0]. So, I have two questions:

* Is there an easier way to get the current database's (or any database's)
OID that I am overlooking (besides "SELECT oid FROM pg_database...")?

* Would anyone object if I put together some patches to add regdatabase?

[0] /messages/by-id/20191109220939.jz55zcc33d3g7h7b@alap3.anarazel.de

Hah, I put together a patch to implement just that a while back, but
then concluded
for some reason that it would likely be rejected so saved myself the
humiliation of
submitting it...

Attaching patch for reference - it's from mid 2020 so no longer applies. I'll
have a crack at cleaning it up if I get a chance.

Regards

Ian Barwick

Attachments:

regdatabase.v1.patchtext/x-patch; charset=US-ASCII; name=regdatabase.v1.patchDownload+250-5
#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Ian Lawrence Barwick (#4)
Re: regdatabase

On Wed, May 07, 2025 at 09:18:28AM +0900, Ian Lawrence Barwick wrote:

Hah, I put together a patch to implement just that a while back, but
then concluded
for some reason that it would likely be rejected so saved myself the
humiliation of
submitting it...

Attaching patch for reference - it's from mid 2020 so no longer applies. I'll
have a crack at cleaning it up if I get a chance.

Oh, thanks!

--
nathan

#6Nathan Bossart
nathandbossart@gmail.com
In reply to: Michael Paquier (#3)
Re: regdatabase

On Wed, May 07, 2025 at 08:45:15AM +0900, Michael Paquier wrote:

For regdatabase, there would be at least two simplications related to
the dump of subscriptions, where we could switch the queries to the
new grammar for backend versions able to support the new grammar,
meaning that we could remove entirely these parts in 11~12 years once
the versions that do not support the new reg shortcut would be out of
scope. :D

Let's not be too hasty...

--
nathan

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#1)
Re: regdatabase

Nathan Bossart <nathandbossart@gmail.com> writes:

* Would anyone object if I put together some patches to add regdatabase?

The original concept of the reg* types was to implement lookups for
cases that are more complicated than "(SELECT oid FROM pg_foo WHERE
fooname = 'whatever')". As an example, regprocedure would be
somewhere between seriously painful and impossible to do by hand.
But any potentially-schema-qualified object name is complicated
enough to justify having a reg* type, in the original vision.

However, we've broken that of late with regnamespace and regrole.
regdatabase would be of exactly the same complexity as those
cases. So I don't see a reason to object, if you think it's
worth the trouble.

regards, tom lane

#8Ian Lawrence Barwick
barwick@gmail.com
In reply to: Nathan Bossart (#5)
Re: regdatabase

2025年5月7日(水) 10:47 Nathan Bossart <nathandbossart@gmail.com>:

On Wed, May 07, 2025 at 09:18:28AM +0900, Ian Lawrence Barwick wrote:

Hah, I put together a patch to implement just that a while back, but
then concluded
for some reason that it would likely be rejected so saved myself the
humiliation of
submitting it...

Attaching patch for reference - it's from mid 2020 so no longer applies. I'll
have a crack at cleaning it up if I get a chance.

Oh, thanks!

Version which applies/builds against current HEAD attached. I haven't
yet had a chance to look at the code beyond fixing it, however.

Regards

Ian Barwick

Attachments:

regdatabase.v2.patchtext/x-patch; charset=US-ASCII; name=regdatabase.v2.patchDownload+238-7
#9Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Nathan Bossart (#1)
Re: regdatabase

On Tue, 6 May 2025 at 21:29, Nathan Bossart <nathandbossart@gmail.com> wrote:

Every once in a while, I find myself wanting to use regdatabase for
something like current_database()::regdatabase, and I'm always surprised
when I inevitably rediscover that it doesn't exist.

+1 for the idea. I keep running into this too.

#10Greg Sabino Mullane
greg@turnstep.com
In reply to: Ian Lawrence Barwick (#8)
Re: regdatabase

On Wed, May 7, 2025 at 4:55 AM Ian Lawrence Barwick <barwick@gmail.com>
wrote:

Version which applies/builds against current HEAD attached. I haven't yet
had a chance to look at the code beyond fixing it, however.

I too, have wanted this over the years, so +1 on the idea.

Quick review: nice patch, very thorough; applies cleanly; all tests pass;
acts as I would expect when testing manually.

Minor pgindent issue in the comment in src/backend/catalog/dependency.c

Would be nice to have test coverage of our special "single dash" case, e.g.

select regdatabase('-')::oid;
0

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

#11Nathan Bossart
nathandbossart@gmail.com
In reply to: Greg Sabino Mullane (#10)
Re: regdatabase

On Wed, May 07, 2025 at 10:29:43AM -0400, Greg Sabino Mullane wrote:

On Wed, May 7, 2025 at 4:55 AM Ian Lawrence Barwick <barwick@gmail.com>
wrote:

Version which applies/builds against current HEAD attached. I haven't yet
had a chance to look at the code beyond fixing it, however.

I spent some time on this one. I saw two main things:

* As of commit aa01051, pg_upgrade does preserve database OIDs, so we don't
need to block for tables with columns of type regdatabase.

* Some of the regproc.c code needs to use the soft error functions instead.

I believe both of these were introduced after you wrote your original patch
in 2020. There have been a couple of refactors, too. The attached patch
should address everything I've mentioned.

Would be nice to have test coverage of our special "single dash" case, e.g.

select regdatabase('-')::oid;
0

Yeah, I'm not sure there's any coverage of that for the existing reg*
types, either.

--
nathan

Attachments:

v3-0001-Add-regdatabase.patchtext/plain; charset=us-asciiDownload+276-4
#12Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#11)
Re: regdatabase

On Wed, May 07, 2025 at 10:41:23PM -0500, Nathan Bossart wrote:

The attached patch should address everything I've mentioned.

And this version of the patch should be free of my embarrassing copy/paste
mistakes.

--
nathan

Attachments:

v4-0001-Add-regdatabase.patchtext/plain; charset=us-asciiDownload+276-4
#13Ian Lawrence Barwick
barwick@gmail.com
In reply to: Nathan Bossart (#11)
Re: regdatabase

2025年5月8日(木) 12:41 Nathan Bossart <nathandbossart@gmail.com>:

On Wed, May 07, 2025 at 10:29:43AM -0400, Greg Sabino Mullane wrote:

On Wed, May 7, 2025 at 4:55 AM Ian Lawrence Barwick <barwick@gmail.com>
wrote:

Version which applies/builds against current HEAD attached. I haven't yet
had a chance to look at the code beyond fixing it, however.

I spent some time on this one. I saw two main things:

* As of commit aa01051, pg_upgrade does preserve database OIDs, so we don't
need to block for tables with columns of type regdatabase.

* Some of the regproc.c code needs to use the soft error functions instead.

I believe both of these were introduced after you wrote your original patch
in 2020. There have been a couple of refactors, too. The attached patch
should address everything I've mentioned.

Thanks for looking at that, I likely wouldn't have got to it for a
couple of days.

Would be nice to have test coverage of our special "single dash" case, e.g.

select regdatabase('-')::oid;
0

Yeah, I'm not sure there's any coverage of that for the existing reg*
types, either.

Revised patch attached which adds coverage of that and also for the
"constant of the type reg(role|database) cannot be used here" error.

Regards

Ian Barwick

Attachments:

v5-0001-Add-regdatabase.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Add-regdatabase.patchDownload+417-4
#14Nathan Bossart
nathandbossart@gmail.com
In reply to: Ian Lawrence Barwick (#13)
Re: regdatabase

On Thu, May 08, 2025 at 10:38:04PM +0900, Ian Lawrence Barwick wrote:

Revised patch attached which adds coverage of that and also for the
"constant of the type reg(role|database) cannot be used here" error.

LGTM. I've marked it as ready-for-committer [0]https://commitfest.postgresql.org/patch/5746/ and will plan on
committing it as soon as v19 development begins.

[0]: https://commitfest.postgresql.org/patch/5746/

--
nathan

#15jian he
jian.universality@gmail.com
In reply to: Nathan Bossart (#14)
Re: regdatabase

On Thu, May 8, 2025 at 11:20 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

On Thu, May 08, 2025 at 10:38:04PM +0900, Ian Lawrence Barwick wrote:

Revised patch attached which adds coverage of that and also for the
"constant of the type reg(role|database) cannot be used here" error.

LGTM. I've marked it as ready-for-committer [0] and will plan on
committing it as soon as v19 development begins.

[0] https://commitfest.postgresql.org/patch/5746/

hi.

I saw REGROLEOID in foreign_expr_walker,
I'm wondering whether REGDATABASEOID is reachable within foreign_expr_walker.

not familiar with this area, also there are no coverage tests for
other reg*Const,
per
https://coverage.postgresql.org/contrib/postgres_fdw/deparse.c.gcov.html

#16Nathan Bossart
nathandbossart@gmail.com
In reply to: jian he (#15)
Re: regdatabase

On Mon, May 19, 2025 at 10:37:29AM +0800, jian he wrote:

I saw REGROLEOID in foreign_expr_walker,
I'm wondering whether REGDATABASEOID is reachable within foreign_expr_walker.

not familiar with this area, also there are no coverage tests for
other reg*Const,
per
https://coverage.postgresql.org/contrib/postgres_fdw/deparse.c.gcov.html

Oh, good catch. At first glance, it looks like we could just add another
case block for REGDATABASEOID, but looking closer at is_shippable(), I see
that it's just testing for objectId < FirstGenbkiObjectId. That seems like
it might be a problem because we don't consider any databases to be pinned,
and template0, template1, and postgres start with OIDs of 4, 1, and 5,
respectively. However, I'd expect that to also be a problem for the public
schema, but foreign_expr_walker() doesn't appear to have any special
handling for that. Hm...

--
nathan

#17Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#16)
Re: regdatabase

On Mon, May 19, 2025 at 04:17:26PM -0500, Nathan Bossart wrote:

On Mon, May 19, 2025 at 10:37:29AM +0800, jian he wrote:

I saw REGROLEOID in foreign_expr_walker,
I'm wondering whether REGDATABASEOID is reachable within foreign_expr_walker.

not familiar with this area, also there are no coverage tests for
other reg*Const,
per
https://coverage.postgresql.org/contrib/postgres_fdw/deparse.c.gcov.html

Oh, good catch. At first glance, it looks like we could just add another
case block for REGDATABASEOID, but looking closer at is_shippable(), I see
that it's just testing for objectId < FirstGenbkiObjectId. That seems like
it might be a problem because we don't consider any databases to be pinned,
and template0, template1, and postgres start with OIDs of 4, 1, and 5,
respectively. However, I'd expect that to also be a problem for the public
schema, but foreign_expr_walker() doesn't appear to have any special
handling for that. Hm...

For now, I've just added another case block for REGDATABASEOID to match the
others. If there are problems with non-pinned objects being considered
shippable, it's not really the fault of this patch. Also, from reading
around [0]/messages/by-id/flat/1423433.1652722406@sss.pgh.pa.us, I get the idea that "shippability" might just mean that the
same object _probably_ exists on the remote server. Plus, there seems to
be very few use-cases for shipping reg* values in the first place. But
even after reading lots of threads, code, and docs, I'm still not sure I
fully grasp all the details here.

[0]: /messages/by-id/flat/1423433.1652722406@sss.pgh.pa.us

--
nathan

Attachments:

v6-0001-Add-regdatabase.patchtext/plain; charset=us-asciiDownload+423-4
#18Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Nathan Bossart (#17)
Re: regdatabase

For now, I've just added another case block for REGDATABASEOID to match

the

others. If there are problems with non-pinned objects being considered
shippable, it's not really the fault of this patch. Also, from reading
around [0], I get the idea that "shippability" might just mean that the
same object _probably_ exists on the remote server. Plus, there seems to
be very few use-cases for shipping reg* values in the first place. But
even after reading lots of threads, code, and docs, I'm still not sure I
fully grasp all the details here.

[0] /messages/by-id/flat/1423433.1652722406@sss.pgh.pa.us

I agree with blocking it for now. The patch LGTM, all tests pass and seems
to cover all the changes.

Not sure if it is worth having some dump/restore tap tests for tables with
regdatabase type.

Regards,

--
Fabrízio de Royes Mello

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#17)
Re: regdatabase

Nathan Bossart <nathandbossart@gmail.com> writes:

For now, I've just added another case block for REGDATABASEOID to match the
others. If there are problems with non-pinned objects being considered
shippable, it's not really the fault of this patch. Also, from reading
around [0], I get the idea that "shippability" might just mean that the
same object _probably_ exists on the remote server. Plus, there seems to
be very few use-cases for shipping reg* values in the first place. But
even after reading lots of threads, code, and docs, I'm still not sure I
fully grasp all the details here.

It's all quite squishy, unfortunately, because shippability is a
heuristic rather than something we can determine with certainty
(at reasonable cost, anyway). But I agree with treating regdatabase
the same as the other reg* types, at least until someone shows up
with a counterexample.

regards, tom lane

#20Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#19)
Re: regdatabase

On Fri, May 30, 2025 at 04:55:58PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

For now, I've just added another case block for REGDATABASEOID to match the
others. If there are problems with non-pinned objects being considered
shippable, it's not really the fault of this patch. Also, from reading
around [0], I get the idea that "shippability" might just mean that the
same object _probably_ exists on the remote server. Plus, there seems to
be very few use-cases for shipping reg* values in the first place. But
even after reading lots of threads, code, and docs, I'm still not sure I
fully grasp all the details here.

It's all quite squishy, unfortunately, because shippability is a
heuristic rather than something we can determine with certainty
(at reasonable cost, anyway). But I agree with treating regdatabase
the same as the other reg* types, at least until someone shows up
with a counterexample.

Got it, thanks for confirming.

--
nathan

#21Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#20)
#22Ian Lawrence Barwick
barwick@gmail.com
In reply to: Nathan Bossart (#21)
#23Nathan Bossart
nathandbossart@gmail.com
In reply to: Ian Lawrence Barwick (#22)