public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/2] bootstrap: convert Typ to a List*
3+ messages / 3 participants
[nested] [flat]
* [PATCH 1/2] bootstrap: convert Typ to a List*
@ 2020-11-20 02:48 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw)
---
src/backend/bootstrap/bootstrap.c | 89 ++++++++++++++-----------------
1 file changed, 41 insertions(+), 48 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6f615e6622..1b940d9d27 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -58,7 +58,7 @@ static void BootstrapModeMain(void);
static void bootstrap_signals(void);
static void ShutdownAuxiliaryProcess(int code, Datum arg);
static Form_pg_attribute AllocateAttribute(void);
-static void populate_typ_array(void);
+static void populate_typ(void);
static Oid gettype(char *type);
static void cleanup(void);
@@ -159,7 +159,7 @@ struct typmap
FormData_pg_type am_typ;
};
-static struct typmap **Typ = NULL;
+static List *Typ = NIL; /* List of struct typmap* */
static struct typmap *Ap = NULL;
static Datum values[MAXATTR]; /* current row's attribute values */
@@ -595,10 +595,10 @@ boot_openrel(char *relname)
/*
* pg_type must be filled before any OPEN command is executed, hence we
- * can now populate the Typ array if we haven't yet.
+ * can now populate Typ if we haven't yet.
*/
- if (Typ == NULL)
- populate_typ_array();
+ if (Typ == NIL)
+ populate_typ();
if (boot_reldesc != NULL)
closerel(NULL);
@@ -688,7 +688,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
typeoid = gettype(type);
- if (Typ != NULL)
+ if (Typ != NIL)
{
attrtypes[attnum]->atttypid = Ap->am_oid;
attrtypes[attnum]->attlen = Ap->am_typ.typlen;
@@ -866,47 +866,36 @@ cleanup(void)
}
/* ----------------
- * populate_typ_array
+ * populate_typ
*
- * Load the Typ array by reading pg_type.
+ * Load Typ by reading pg_type.
* ----------------
*/
static void
-populate_typ_array(void)
+populate_typ(void)
{
Relation rel;
TableScanDesc scan;
HeapTuple tup;
- int nalloc;
- int i;
-
- Assert(Typ == NULL);
+ MemoryContext old;
- nalloc = 512;
- Typ = (struct typmap **)
- MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+ Assert(Typ == NIL);
rel = table_open(TypeRelationId, NoLock);
scan = table_beginscan_catalog(rel, 0, NULL);
- i = 0;
+ old = MemoryContextSwitchTo(TopMemoryContext);
while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+ struct typmap *newtyp;
- /* make sure there will be room for a trailing NULL pointer */
- if (i >= nalloc - 1)
- {
- nalloc *= 2;
- Typ = (struct typmap **)
- repalloc(Typ, nalloc * sizeof(struct typmap *));
- }
- Typ[i] = (struct typmap *)
- MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
- Typ[i]->am_oid = typForm->oid;
- memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
- i++;
+ newtyp = (struct typmap *) palloc(sizeof(struct typmap));
+ Typ = lappend(Typ, newtyp);
+
+ newtyp->am_oid = typForm->oid;
+ memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ));
}
- Typ[i] = NULL; /* Fill trailing NULL pointer */
+ MemoryContextSwitchTo(old);
table_endscan(scan);
table_close(rel, NoLock);
}
@@ -916,25 +905,26 @@ populate_typ_array(void)
*
* NB: this is really ugly; it will return an integer index into TypInfo[],
* and not an OID at all, until the first reference to a type not known in
- * TypInfo[]. At that point it will read and cache pg_type in the Typ array,
+ * TypInfo[]. At that point it will read and cache pg_type in Typ,
* and subsequently return a real OID (and set the global pointer Ap to
* point at the found row in Typ). So caller must check whether Typ is
- * still NULL to determine what the return value is!
+ * still NIL to determine what the return value is!
* ----------------
*/
static Oid
gettype(char *type)
{
- if (Typ != NULL)
+ if (Typ != NIL)
{
- struct typmap **app;
+ ListCell *lc;
- for (app = Typ; *app != NULL; app++)
+ foreach (lc, Typ)
{
- if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
+ struct typmap *app = lfirst(lc);
+ if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
{
- Ap = *app;
- return (*app)->am_oid;
+ Ap = app;
+ return app->am_oid;
}
}
}
@@ -949,7 +939,7 @@ gettype(char *type)
}
/* Not in TypInfo, so we'd better be able to read pg_type now */
elog(DEBUG4, "external type: %s", type);
- populate_typ_array();
+ populate_typ();
return gettype(type);
}
elog(ERROR, "unrecognized type \"%s\"", type);
@@ -977,17 +967,20 @@ boot_get_type_io_data(Oid typid,
Oid *typinput,
Oid *typoutput)
{
- if (Typ != NULL)
+ if (Typ != NIL)
{
/* We have the boot-time contents of pg_type, so use it */
- struct typmap **app;
- struct typmap *ap;
-
- app = Typ;
- while (*app && (*app)->am_oid != typid)
- ++app;
- ap = *app;
- if (ap == NULL)
+ struct typmap *ap = NULL;
+ ListCell *lc;
+
+ foreach (lc, Typ)
+ {
+ ap = lfirst(lc);
+ if (ap->am_oid == typid)
+ break;
+ }
+
+ if (!ap || ap->am_oid != typid)
elog(ERROR, "type OID %u not found in Typ list", typid);
*typlen = ap->am_typ.typlen;
--
2.17.0
--yQbNiKLmgenwUfTN
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0002-Allow-composite-types-in-bootstrap.patchx"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: confusing results from pg_get_replication_slots()
@ 2026-01-02 20:48 Matheus Alcantara <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Matheus Alcantara @ 2026-01-02 20:48 UTC (permalink / raw)
To: Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On 02/01/26 12:40, Robert Haas wrote:
> Here is a patch to make invalidated slots always report as "lost",
> which I propose to back-patch to all supported versions.
>
The patch looks correct to me. I'm just wondering if/how we could
create a test for this. It possible to create a regression test or a
TAP test? Or it's not worthwhile?
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: confusing results from pg_get_replication_slots()
@ 2026-01-02 21:10 Robert Haas <[email protected]>
parent: Matheus Alcantara <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Robert Haas @ 2026-01-02 21:10 UTC (permalink / raw)
To: Matheus Alcantara <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Fri, Jan 2, 2026 at 3:48 PM Matheus Alcantara
<[email protected]> wrote:
> On 02/01/26 12:40, Robert Haas wrote:
> > Here is a patch to make invalidated slots always report as "lost",
> > which I propose to back-patch to all supported versions.
>
> The patch looks correct to me. I'm just wondering if/how we could
> create a test for this. It possible to create a regression test or a
> TAP test? Or it's not worthwhile?
It's relatively difficult to reproduce this, especially on master.
Amit Kapila's commit f41d8468ddea34170fe19fdc17b5a247e7d3ac78 changed
the behavior for physical replication slots. Before that commit, you
couldn't connect to an invalidated logical replication slot, but not
an invalidated physical replication slot. After this commit, both are
prohibited. I imagine that Amit thought this was a distinction without
a difference, because of course if the WAL is actually removed then
use of the slot will fail later -- but that's not completely true,
because there's no guarantee if or when the connection will be used to
fetch WAL that has been removed. Nonetheless, I think it's a good
change: because invalidated replication slots are ignored, having
stuff connect to them and pretend to use them is bad.
However, this means that if you wanted a TAP test for this, you would
have to let a replication slot get behind far enough that it could be
invalidated, trigger a checkpoint that actually invalidates it, and
then have the process using the connection catch up quickly enough
that it never tries to fetch removed WAL. In older releases, I believe
it's a little easier to hit the problem, because you can actually
reconnect to an invalidated slot, but I think you still need to the
timing to be just right, so that you catch up after the invalidation
happens but before the files are actually removed. Even there, I don't
see how you could construct a TAP test without injection points, and
I'm not really convinced that it's worth adding a bunch of new
infrastructure for this. Such a test wouldn't be likely to catch the
next bug of this type, if there is one.
The best thing to do to really avoid future bugs of this type, IMHO,
would be to modify pg_get_replication_slots() so that it does not
editorialize on the value returned by GetWALAvaliability(), but how to
get there is arguable. Maybe we shouldn't display "lost" when the slot
is invalidated but "invalidated", for example, and any other value
means we're just returning whatever GetWALAvaliability() told us.
Also, maybe the exception for connect slots should just be removed, on
the assumption that the race condition isn't common enough to matter,
or maybe that logic should be pushed down into GetWALAvailability() if
we want to keep it. I'm not sure. Any of that seems like too much to
change in the back-branches, but I personally believe rethinking the
logic here would be a better use of energy than developing test cases
that verify the exact details of the current logic.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-01-02 21:10 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 02:48 [PATCH 1/2] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]>
2026-01-02 20:48 Re: confusing results from pg_get_replication_slots() Matheus Alcantara <[email protected]>
2026-01-02 21:10 ` Re: confusing results from pg_get_replication_slots() Robert Haas <[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