From: Justin Pryzby <pryzby@telsasoft.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: PoC/WIP: Extended statistics on expressions
Date: Thu, 4 Mar 2021 18:43:35 -0600
Message-ID: <20210305004334.GR29832@telsasoft.com> (raw)
In-Reply-To: <20210104154524.GE9712@telsasoft.com>
References: <6455e0ad-2a2e-bdb0-e1f8-fd4983d1c8aa@enterprisedb.com>
<087086de-1384-8374-4efc-2e7915071d5c@enterprisedb.com>
<CAEZATCU9uPo7JYdx4k0-ufXXZH8t7itodibUwCva+s+AvAKcnw@mail.gmail.com>
<b2995773-c9a1-5d3b-fc90-4f3ab189be11@enterprisedb.com>
<CAEZATCWmQOFUqo4GXPhGsYGrbhYq0J4ZMz8B-LZXptu7S2y2GQ@mail.gmail.com>
<4a2918bb-49b0-ccd5-3af7-f7795941a6ad@enterprisedb.com>
<CAEZATCUiy-Vtj9RfVrTMCQpfVO_VaCD9Dk4jBj0j_eEFEBS1ow@mail.gmail.com>
<958870c8-65e0-31b1-4591-b0b10e807dd9@enterprisedb.com>
<CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb+rQ@mail.gmail.com>
<20210104154524.GE9712@telsasoft.com>
On Mon, Jan 04, 2021 at 09:45:24AM -0600, Justin Pryzby wrote:
> On Mon, Jan 04, 2021 at 03:34:08PM +0000, Dean Rasheed wrote:
> > * I'm not sure I understand the need for 0001. Wasn't there an earlier
> > version of this patch that just did it by re-populating the type
> > array, but which still had it as an array rather than turning it into
> > a list? Making it a list falsifies some of the comments and
> > function/variable name choices in that file.
>
> This part is from me.
>
> I can review the names if it's desired , but it'd be fine to fall back to the
> earlier patch. I thought a pglist was cleaner, but it's not needed.
This updates the preliminary patches to address the issues Dean raised.
One advantage of using a pglist is that we can free it by calling
list_free_deep(Typ), rather than looping to free each of its elements.
But maybe for bootstrap.c it doesn't matter, and we can just write:
| Typ = NULL; /* Leak the old Typ array */
--
Justin
Attachments:
[text/x-diff] 0001-bootstrap-convert-Typ-to-a-List.patchx (5.2K, ../20210305004334.GR29832@telsasoft.com/2-0001-bootstrap-convert-Typ-to-a-List.patchx)
download | inline diff:
From 41ec12096cefc00484e7f2a0b3bfbc0f79cdd162 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 19 Nov 2020 20:48:48 -0600
Subject: [PATCH 1/2] bootstrap: convert Typ to a List*
---
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.cindex 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
[text/x-diff] 0002-Allow-composite-types-in-bootstrap.patchx (1.4K, ../20210305004334.GR29832@telsasoft.com/3-0002-Allow-composite-types-in-bootstrap.patchx)
download | inline diff:
From 25c8324e16ffdaa3ea4eedee95bb76257964a540 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 17 Nov 2020 09:28:33 -0600
Subject: [PATCH 2/2] Allow composite types in bootstrap
---
src/backend/bootstrap/bootstrap.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, dean.a.rasheed@gmail.com, tomas.vondra@enterprisedb.com
Subject: Re: PoC/WIP: Extended statistics on expressions
In-Reply-To: <20210305004334.GR29832@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox