public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 2/4] Allow composite types in bootstrap
10+ messages / 6 participants
[nested] [flat]

* [PATCH 2/4] Allow composite types in bootstrap
@ 2020-11-17 15:28 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Justin Pryzby @ 2020-11-17 15:28 UTC (permalink / raw)

---
 src/backend/bootstrap/bootstrap.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 1b940d9d27..a0fcbb3d83 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -916,6 +916,7 @@ gettype(char *type)
 {
 	if (Typ != NIL)
 	{
+		static bool did_reread PG_USED_FOR_ASSERTS_ONLY = false; /* Already reread pg_types? */
 		ListCell *lc;
 
 		foreach (lc, Typ)
@@ -927,6 +928,33 @@ gettype(char *type)
 				return app->am_oid;
 			}
 		}
+
+		/*
+		 * The type wasn't known; check again to handle composite
+		 * types, added since first populating Typ.
+		 */
+
+		/*
+		 * Once all the types are populated and we handled composite
+		 * types, shouldn't need to do that again.
+		 */
+		Assert(!did_reread);
+		did_reread = true;
+
+		list_free_deep(Typ);
+		Typ = NIL;
+		populate_typ();
+
+		/* Need to avoid infinite recursion... */
+		foreach (lc, Typ)
+		{
+			struct typmap *app = lfirst(lc);
+			if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
+			{
+				Ap = app;
+				return app->am_oid;
+			}
+		}
 	}
 	else
 	{
-- 
2.26.2


--------------BCA1373AE29B32B57608F4EE
Content-Type: text/x-patch; charset=UTF-8;
 name="0003-Use-correct-statistics-kind-in-a-couple-pla-20210307.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0003-Use-correct-statistics-kind-in-a-couple-pla-20210307.pa";
 filename*1="tch"



^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Obsolete reference to pg_relation in comment
@ 2023-07-26 17:48 Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 18:53 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  0 siblings, 2 replies; 10+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2023-07-26 17:48 UTC (permalink / raw)
  To: pgsql-hackers

Hi hackers,

Triggered by a discussion on IRC, I noticed that there's a stray
reference to pg_relation in a comment that was added long after it was
renamed to pg_class.  Here's a patch to bring that up to speed.

- ilmari



Attachments:

  [text/x-diff] 0001-Fix-obsolete-reference-to-pg_relation-in-comment.patch (1.2K, ../../[email protected]/2-0001-Fix-obsolete-reference-to-pg_relation-in-comment.patch)
  download | inline diff:
From e395f8cb293f674f45eb3847534de07c7124e738 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <[email protected]>
Date: Wed, 26 Jul 2023 18:31:51 +0100
Subject: [PATCH] Fix obsolete reference to pg_relation in comment

pg_relation was renamed to pg_class in 1991, but this comment (added
in 2004) missed the memo
---
 src/backend/storage/large_object/inv_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c
index 84e543e731..a56912700b 100644
--- a/src/backend/storage/large_object/inv_api.c
+++ b/src/backend/storage/large_object/inv_api.c
@@ -59,7 +59,7 @@ bool		lo_compat_privileges;
 
 /*
  * All accesses to pg_largeobject and its index make use of a single Relation
- * reference, so that we only need to open pg_relation once per transaction.
+ * reference, so that we only need to open pg_class once per transaction.
  * To avoid problems when the first such reference occurs inside a
  * subtransaction, we execute a slightly klugy maneuver to assign ownership of
  * the Relation reference to TopTransactionResourceOwner.
-- 
2.39.2



^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
@ 2023-07-26 18:53 ` Nathan Bossart <[email protected]>
  2023-07-26 19:11   ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  1 sibling, 1 reply; 10+ messages in thread

From: Nathan Bossart @ 2023-07-26 18:53 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: pgsql-hackers

On Wed, Jul 26, 2023 at 06:48:51PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Triggered by a discussion on IRC, I noticed that there's a stray
> reference to pg_relation in a comment that was added long after it was
> renamed to pg_class.  Here's a patch to bring that up to speed.

> pg_relation was renamed to pg_class in 1991, but this comment (added
> in 2004) missed the memo

Huh, interesting!  I dug around the Berkeley archives [0] and found
comments indicating that pg_relation was renamed to pg_class in Februrary
1990.  However, it looks like the file was named pg_relation.h until
Postgres95 v0.01, which has the following comment in pg_class.h:

	 *    ``pg_relation'' is being replaced by ``pg_class''.  currently
	 *    we are only changing the name in the catalogs but someday the
	 *    code will be changed too. -cim 2/26/90
	 *    [it finally happens.  -ay 11/5/94]

[0] https://dsf.berkeley.edu/postgres.html

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 18:53 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
@ 2023-07-26 19:11   ` Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Nathan Bossart @ 2023-07-26 19:11 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: pgsql-hackers

On Wed, Jul 26, 2023 at 11:53:06AM -0700, Nathan Bossart wrote:
> On Wed, Jul 26, 2023 at 06:48:51PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Triggered by a discussion on IRC, I noticed that there's a stray
>> reference to pg_relation in a comment that was added long after it was
>> renamed to pg_class.  Here's a patch to bring that up to speed.
> 
>> pg_relation was renamed to pg_class in 1991, but this comment (added
>> in 2004) missed the memo
> 
> Huh, interesting!  I dug around the Berkeley archives [0] and found
> comments indicating that pg_relation was renamed to pg_class in Februrary
> 1990.  However, it looks like the file was named pg_relation.h until
> Postgres95 v0.01, which has the following comment in pg_class.h:
> 
> 	 *    ``pg_relation'' is being replaced by ``pg_class''.  currently
> 	 *    we are only changing the name in the catalogs but someday the
> 	 *    code will be changed too. -cim 2/26/90
> 	 *    [it finally happens.  -ay 11/5/94]

This comment actually lived in Postgres until 9cf80f2 (June 2000), too.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
@ 2023-07-26 20:50 ` Nathan Bossart <[email protected]>
  2023-07-26 21:14   ` Re: Obsolete reference to pg_relation in comment Tom Lane <[email protected]>
  1 sibling, 1 reply; 10+ messages in thread

From: Nathan Bossart @ 2023-07-26 20:50 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: pgsql-hackers

Okay, now looking at the patch...

On Wed, Jul 26, 2023 at 06:48:51PM +0100, Dagfinn Ilmari Mannsåker wrote:
>   * All accesses to pg_largeobject and its index make use of a single Relation
> - * reference, so that we only need to open pg_relation once per transaction.
> + * reference, so that we only need to open pg_class once per transaction.
>   * To avoid problems when the first such reference occurs inside a
>   * subtransaction, we execute a slightly klugy maneuver to assign ownership of
>   * the Relation reference to TopTransactionResourceOwner.

Hm.  Are you sure this is actually referring to pg_class?  It seems
unlikely given pg_relation was renamed 14 years before this comment was
added, and the code appears to be ensuring that pg_largeobject and its
index are opened at most once per transaction.  I couldn't find the
original thread for this comment, unfortunately, but ISTM we might want to
replace "pg_relation" with "them" instead.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
@ 2023-07-26 21:14   ` Tom Lane <[email protected]>
  2023-07-26 21:20     ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  2023-09-06 19:13     ` Re: Obsolete reference to pg_relation in comment Bruce Momjian <[email protected]>
  0 siblings, 2 replies; 10+ messages in thread

From: Tom Lane @ 2023-07-26 21:14 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Dagfinn Ilmari Mannsåker <[email protected]>; pgsql-hackers

Nathan Bossart <[email protected]> writes:
> On Wed, Jul 26, 2023 at 06:48:51PM +0100, Dagfinn Ilmari Mannsåker wrote:
>>   * All accesses to pg_largeobject and its index make use of a single Relation
>> - * reference, so that we only need to open pg_relation once per transaction.
>> + * reference, so that we only need to open pg_class once per transaction.
>>   * To avoid problems when the first such reference occurs inside a
>>   * subtransaction, we execute a slightly klugy maneuver to assign ownership of
>>   * the Relation reference to TopTransactionResourceOwner.

> Hm.  Are you sure this is actually referring to pg_class?  It seems
> unlikely given pg_relation was renamed 14 years before this comment was
> added, and the code appears to be ensuring that pg_largeobject and its
> index are opened at most once per transaction.

I believe it is just a typo/thinko for pg_class, but there's more not
to like about this comment.  First, once we've made a relcache entry
it would typically stay valid across uses, so it's far from clear that
this coding actually prevents many catalog accesses in typical cases.
Second, when we do have to rebuild the relcache entry, there's a lot
more involved than just a pg_class fetch; we at least need to read
pg_attribute, and I think there may be other catalogs that we'd read
along the way, even for a system catalog that lacks complicated
features.  (pg_index would presumably get looked at, for instance.)

I think we should reword this to just generically claim that holding
the Relation reference open for the whole transaction reduces overhead.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  2023-07-26 21:14   ` Re: Obsolete reference to pg_relation in comment Tom Lane <[email protected]>
@ 2023-07-26 21:20     ` Nathan Bossart <[email protected]>
  1 sibling, 0 replies; 10+ messages in thread

From: Nathan Bossart @ 2023-07-26 21:20 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Dagfinn Ilmari Mannsåker <[email protected]>; pgsql-hackers

On Wed, Jul 26, 2023 at 05:14:08PM -0400, Tom Lane wrote:
> I think we should reword this to just generically claim that holding
> the Relation reference open for the whole transaction reduces overhead.

WFM

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  2023-07-26 21:14   ` Re: Obsolete reference to pg_relation in comment Tom Lane <[email protected]>
@ 2023-09-06 19:13     ` Bruce Momjian <[email protected]>
  2023-09-07 08:44       ` Re: Obsolete reference to pg_relation in comment Daniel Gustafsson <[email protected]>
  1 sibling, 1 reply; 10+ messages in thread

From: Bruce Momjian @ 2023-09-06 19:13 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; pgsql-hackers

On Wed, Jul 26, 2023 at 05:14:08PM -0400, Tom Lane wrote:
> Nathan Bossart <[email protected]> writes:
> > On Wed, Jul 26, 2023 at 06:48:51PM +0100, Dagfinn Ilmari Mannsåker wrote:
> >>   * All accesses to pg_largeobject and its index make use of a single Relation
> >> - * reference, so that we only need to open pg_relation once per transaction.
> >> + * reference, so that we only need to open pg_class once per transaction.
> >>   * To avoid problems when the first such reference occurs inside a
> >>   * subtransaction, we execute a slightly klugy maneuver to assign ownership of
> >>   * the Relation reference to TopTransactionResourceOwner.
> 
> > Hm.  Are you sure this is actually referring to pg_class?  It seems
> > unlikely given pg_relation was renamed 14 years before this comment was
> > added, and the code appears to be ensuring that pg_largeobject and its
> > index are opened at most once per transaction.
> 
> I believe it is just a typo/thinko for pg_class, but there's more not
> to like about this comment.  First, once we've made a relcache entry
> it would typically stay valid across uses, so it's far from clear that
> this coding actually prevents many catalog accesses in typical cases.
> Second, when we do have to rebuild the relcache entry, there's a lot
> more involved than just a pg_class fetch; we at least need to read
> pg_attribute, and I think there may be other catalogs that we'd read
> along the way, even for a system catalog that lacks complicated
> features.  (pg_index would presumably get looked at, for instance.)
> 
> I think we should reword this to just generically claim that holding
> the Relation reference open for the whole transaction reduces overhead.

How is this attached patch?

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.


Attachments:

  [text/x-diff] pg_relation.diff (1.2K, ../../[email protected]/2-pg_relation.diff)
  download | inline diff:
diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c
new file mode 100644
index 84e543e..cc9c335
*** a/src/backend/storage/large_object/inv_api.c
--- b/src/backend/storage/large_object/inv_api.c
***************
*** 58,68 ****
  bool		lo_compat_privileges;
  
  /*
!  * All accesses to pg_largeobject and its index make use of a single Relation
!  * reference, so that we only need to open pg_relation once per transaction.
!  * To avoid problems when the first such reference occurs inside a
!  * subtransaction, we execute a slightly klugy maneuver to assign ownership of
!  * the Relation reference to TopTransactionResourceOwner.
   */
  static Relation lo_heap_r = NULL;
  static Relation lo_index_r = NULL;
--- 58,68 ----
  bool		lo_compat_privileges;
  
  /*
!  * All accesses to pg_largeobject and its index make use of a single
!  * Relation reference.  To guarantee that the relcache entry remains
!  * in the cache, on the first reference inside a subtransaction, we
!  * execute a slightly klugy maneuver to assign ownership of the
!  * Relation reference to TopTransactionResourceOwner.
   */
  static Relation lo_heap_r = NULL;
  static Relation lo_index_r = NULL;


^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  2023-07-26 21:14   ` Re: Obsolete reference to pg_relation in comment Tom Lane <[email protected]>
  2023-09-06 19:13     ` Re: Obsolete reference to pg_relation in comment Bruce Momjian <[email protected]>
@ 2023-09-07 08:44       ` Daniel Gustafsson <[email protected]>
  2023-09-26 21:07         ` Re: Obsolete reference to pg_relation in comment Bruce Momjian <[email protected]>
  0 siblings, 1 reply; 10+ messages in thread

From: Daniel Gustafsson @ 2023-09-07 08:44 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; pgsql-hackers

> On 6 Sep 2023, at 21:13, Bruce Momjian <[email protected]> wrote:
> On Wed, Jul 26, 2023 at 05:14:08PM -0400, Tom Lane wrote:

>> I think we should reword this to just generically claim that holding
>> the Relation reference open for the whole transaction reduces overhead.
> 
> How is this attached patch?

Reads good to me, +1.

--
Daniel Gustafsson







^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Obsolete reference to pg_relation in comment
  2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
  2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
  2023-07-26 21:14   ` Re: Obsolete reference to pg_relation in comment Tom Lane <[email protected]>
  2023-09-06 19:13     ` Re: Obsolete reference to pg_relation in comment Bruce Momjian <[email protected]>
  2023-09-07 08:44       ` Re: Obsolete reference to pg_relation in comment Daniel Gustafsson <[email protected]>
@ 2023-09-26 21:07         ` Bruce Momjian <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Bruce Momjian @ 2023-09-26 21:07 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; pgsql-hackers

On Thu, Sep  7, 2023 at 10:44:25AM +0200, Daniel Gustafsson wrote:
> > On 6 Sep 2023, at 21:13, Bruce Momjian <[email protected]> wrote:
> > On Wed, Jul 26, 2023 at 05:14:08PM -0400, Tom Lane wrote:
> 
> >> I think we should reword this to just generically claim that holding
> >> the Relation reference open for the whole transaction reduces overhead.
> > 
> > How is this attached patch?
> 
> Reads good to me, +1.

Patch applied to master.  I didn't think backpatching it made much sense
since it is so localized.

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.






^ permalink  raw  reply  [nested|flat] 10+ messages in thread


end of thread, other threads:[~2023-09-26 21:07 UTC | newest]

Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 15:28 [PATCH 2/4] Allow composite types in bootstrap Justin Pryzby <[email protected]>
2023-07-26 17:48 Obsolete reference to pg_relation in comment Dagfinn Ilmari Mannsåker <[email protected]>
2023-07-26 18:53 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
2023-07-26 19:11   ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
2023-07-26 20:50 ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
2023-07-26 21:14   ` Re: Obsolete reference to pg_relation in comment Tom Lane <[email protected]>
2023-07-26 21:20     ` Re: Obsolete reference to pg_relation in comment Nathan Bossart <[email protected]>
2023-09-06 19:13     ` Re: Obsolete reference to pg_relation in comment Bruce Momjian <[email protected]>
2023-09-07 08:44       ` Re: Obsolete reference to pg_relation in comment Daniel Gustafsson <[email protected]>
2023-09-26 21:07         ` Re: Obsolete reference to pg_relation in comment Bruce Momjian <[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