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

* [PATCH 2/2] Allow composite types in bootstrap
@ 2020-11-17 15:28  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 3+ 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.17.0


--yQbNiKLmgenwUfTN--





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

* Re: PublicationActions - use bit flags.
@ 2022-01-21 00:05  Greg Nancarrow <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Greg Nancarrow @ 2022-01-21 00:05 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 12:55 PM Greg Nancarrow <[email protected]> wrote:
>
> On Tue, Dec 21, 2021 at 11:56 AM Tom Lane <[email protected]> wrote:
> >
> > Removing this is not good:
> >
> >         if (relation->rd_pubactions)
> > -       {
> >                 pfree(relation->rd_pubactions);
> > -               relation->rd_pubactions = NULL;
> > -       }
> >
> > If the subsequent palloc fails, you've created a problem where
> > there was none before.
> >
>
> Oops, yeah, I got carried away; if palloc() failed and called exit(),
> then it would end up crashing when trying to use/pfree rd_pubactions
> again.
> Better leave that line in ...
>

Attaching an updated patch to fix that oversight.
This patch thus fixes the original palloc issue in a minimal way,
keeping the same relcache structure.

Regards,
Greg Nancarrow
Fujitsu Australia


Attachments:

  [application/octet-stream] v2_get_rel_pubactions_improvement.patch (3.9K, ../../CAJcOf-d0=vQx1Pzbf+LVarywejJFS5W+M6uR+2d0oeEJ2VQ+Ew@mail.gmail.com/2-v2_get_rel_pubactions_improvement.patch)
  download | inline diff:
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 313c87398b..f68bcde02b 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -567,7 +567,7 @@ ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo,
 void
 CheckCmdReplicaIdentity(Relation rel, CmdType cmd)
 {
-	PublicationActions *pubactions;
+	PublicationActions pubactions;
 
 	/* We only need to do checks for UPDATE and DELETE. */
 	if (cmd != CMD_UPDATE && cmd != CMD_DELETE)
@@ -583,14 +583,14 @@ CheckCmdReplicaIdentity(Relation rel, CmdType cmd)
 	 *
 	 * Check if the table publishes UPDATES or DELETES.
 	 */
-	pubactions = GetRelationPublicationActions(rel);
-	if (cmd == CMD_UPDATE && pubactions->pubupdate)
+	GetRelationPublicationActions(rel, &pubactions);
+	if (cmd == CMD_UPDATE && pubactions.pubupdate)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("cannot update table \"%s\" because it does not have a replica identity and publishes updates",
 						RelationGetRelationName(rel)),
 				 errhint("To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.")));
-	else if (cmd == CMD_DELETE && pubactions->pubdelete)
+	else if (cmd == CMD_DELETE && pubactions.pubdelete)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("cannot delete from table \"%s\" because it does not have a replica identity and publishes deletes",
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 2e760e8a3b..be05f6cf80 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5524,25 +5524,29 @@ RelationGetExclusionInfo(Relation indexRelation,
 /*
  * Get publication actions for the given relation.
  */
-struct PublicationActions *
-GetRelationPublicationActions(Relation relation)
+void
+GetRelationPublicationActions(Relation relation, PublicationActions *pubactions)
 {
 	List	   *puboids;
 	ListCell   *lc;
 	MemoryContext oldcxt;
 	Oid			schemaid;
-	PublicationActions *pubactions = palloc0(sizeof(PublicationActions));
 
 	/*
 	 * If not publishable, it publishes no actions.  (pgoutput_change() will
 	 * ignore it.)
 	 */
 	if (!is_publishable_relation(relation))
-		return pubactions;
+	{
+		memset(pubactions, 0, sizeof(*pubactions));
+		return;
+	}
 
 	if (relation->rd_pubactions)
-		return memcpy(pubactions, relation->rd_pubactions,
-					  sizeof(PublicationActions));
+	{
+		*pubactions = *relation->rd_pubactions;
+		return;
+	}
 
 	/* Fetch the publication membership info. */
 	puboids = GetRelationPublications(RelationGetRelid(relation));
@@ -5568,6 +5572,7 @@ GetRelationPublicationActions(Relation relation)
 	}
 	puboids = list_concat_unique_oid(puboids, GetAllTablesPublications());
 
+	memset(pubactions, 0, sizeof(*pubactions));
 	foreach(lc, puboids)
 	{
 		Oid			pubid = lfirst_oid(lc);
@@ -5606,10 +5611,8 @@ GetRelationPublicationActions(Relation relation)
 	/* Now save copy of the actions in the relcache entry. */
 	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
 	relation->rd_pubactions = palloc(sizeof(PublicationActions));
-	memcpy(relation->rd_pubactions, pubactions, sizeof(PublicationActions));
+	*relation->rd_pubactions = *pubactions;
 	MemoryContextSwitchTo(oldcxt);
-
-	return pubactions;
 }
 
 /*
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 84d6afef19..46d9db203e 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -75,7 +75,8 @@ extern void RelationInitIndexAccessInfo(Relation relation);
 
 /* caller must include pg_publication.h */
 struct PublicationActions;
-extern struct PublicationActions *GetRelationPublicationActions(Relation relation);
+extern void GetRelationPublicationActions(Relation relation,
+										  struct PublicationActions *pubactions);
 
 extern void RelationInitTableAccessMethod(Relation relation);
 


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

* Re: PublicationActions - use bit flags.
@ 2022-01-24 20:31  Peter Eisentraut <[email protected]>
  parent: Greg Nancarrow <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Peter Eisentraut @ 2022-01-24 20:31 UTC (permalink / raw)
  To: Greg Nancarrow <[email protected]>; Tom Lane <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>


On 21.01.22 01:05, Greg Nancarrow wrote:
> On Tue, Dec 21, 2021 at 12:55 PM Greg Nancarrow <[email protected]> wrote:
>>
>> On Tue, Dec 21, 2021 at 11:56 AM Tom Lane <[email protected]> wrote:
>>>
>>> Removing this is not good:
>>>
>>>          if (relation->rd_pubactions)
>>> -       {
>>>                  pfree(relation->rd_pubactions);
>>> -               relation->rd_pubactions = NULL;
>>> -       }
>>>
>>> If the subsequent palloc fails, you've created a problem where
>>> there was none before.
>>>
>>
>> Oops, yeah, I got carried away; if palloc() failed and called exit(),
>> then it would end up crashing when trying to use/pfree rd_pubactions
>> again.
>> Better leave that line in ...
>>
> 
> Attaching an updated patch to fix that oversight.
> This patch thus fixes the original palloc issue in a minimal way,
> keeping the same relcache structure.

Why can't GetRelationPublicationActions() have the PublicationActions as 
a return value, instead of changing it to an output argument?






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


end of thread, other threads:[~2022-01-24 20:31 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 15:28 [PATCH 2/2] Allow composite types in bootstrap Justin Pryzby <[email protected]>
2022-01-21 00:05 Re: PublicationActions - use bit flags. Greg Nancarrow <[email protected]>
2022-01-24 20:31 ` Re: PublicationActions - use bit flags. Peter Eisentraut <[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