public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 1/3] bootstrap: convert Typ to a List* 2+ messages / 2 participants [nested] [flat]
* [PATCH 1/3] bootstrap: convert Typ to a List* @ 2020-11-20 02:48 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw) --- src/backend/bootstrap/bootstrap.c | 69 ++++++++++++++----------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 6f615e6622..18eb62ca47 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -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 */ @@ -597,7 +597,7 @@ 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. */ - if (Typ == NULL) + if (Typ == NIL) populate_typ_array(); if (boot_reldesc != 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; @@ -877,36 +877,25 @@ populate_typ_array(void) Relation rel; TableScanDesc scan; HeapTuple tup; - int nalloc; - int i; - - Assert(Typ == NULL); - 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; while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL) { Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup); + struct typmap *newtyp; + MemoryContext old; - /* 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++; + old = MemoryContextSwitchTo(TopMemoryContext); + newtyp = (struct typmap *) palloc(sizeof(struct typmap)); + Typ = lappend(Typ, newtyp); + MemoryContextSwitchTo(old); + + newtyp->am_oid = typForm->oid; + memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ)); } - Typ[i] = NULL; /* Fill trailing NULL pointer */ table_endscan(scan); table_close(rel, NoLock); } @@ -925,16 +914,17 @@ populate_typ_array(void) 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; } } } @@ -980,14 +970,17 @@ boot_get_type_io_data(Oid typid, if (Typ != NULL) { /* 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.26.2 --------------76B8D0DC8AE327D516738282 Content-Type: text/x-patch; charset=UTF-8; name="0002-Allow-composite-types-in-bootstrap-20210108.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Allow-composite-types-in-bootstrap-20210108.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Rethinking parallel-scan relation identity checks @ 2024-09-19 23:45 Tom Lane <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Tom Lane @ 2024-09-19 23:45 UTC (permalink / raw) To: [email protected] In [1] I whined about how the parallel heap scan machinery should have noticed that the same ParallelTableScanDesc was being used to give out block numbers for two different relations. Looking closer, there are Asserts that mean to catch this type of error --- but they are comparing relation OIDs, whereas what would have been needed to detect the problem was to compare RelFileLocators. It seems to me that a scan is fundamentally operating at the physical relation level, and therefore these tests should check RelFileLocators not OIDs. Hence I propose the attached. (For master only, of course; this would be an ABI break in the back branches.) This passes check-world and is able to catch the problem exposed in the other thread. Another possible view is that we should check both physical and logical relation IDs, but that seems like overkill to me. Thoughts? regards, tom lane [1] https://www.postgresql.org/message-id/2042942.1726781733%40sss.pgh.pa.us Attachments: [text/x-diff] check-parallel-scan-locators-not-OIDs.patch (3.4K, ../../[email protected]/2-check-parallel-scan-locators-not-OIDs.patch) download | inline diff: diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index dcd04b813d..1859be614c 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -500,8 +500,8 @@ index_parallelscan_initialize(Relation heapRelation, Relation indexRelation, EstimateSnapshotSpace(snapshot)); offset = MAXALIGN(offset); - target->ps_relid = RelationGetRelid(heapRelation); - target->ps_indexid = RelationGetRelid(indexRelation); + target->ps_locator = heapRelation->rd_locator; + target->ps_indexlocator = indexRelation->rd_locator; target->ps_offset = offset; SerializeSnapshot(snapshot, target->ps_snapshot_data); @@ -544,7 +544,9 @@ index_beginscan_parallel(Relation heaprel, Relation indexrel, int nkeys, Snapshot snapshot; IndexScanDesc scan; - Assert(RelationGetRelid(heaprel) == pscan->ps_relid); + Assert(RelFileLocatorEquals(heaprel->rd_locator, pscan->ps_locator)); + Assert(RelFileLocatorEquals(indexrel->rd_locator, pscan->ps_indexlocator)); + snapshot = RestoreSnapshot(pscan->ps_snapshot_data); RegisterSnapshot(snapshot); scan = index_beginscan_internal(indexrel, nkeys, norderbys, snapshot, diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index e57a0b7ea3..bd8715b679 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -168,7 +168,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan) uint32 flags = SO_TYPE_SEQSCAN | SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE; - Assert(RelationGetRelid(relation) == pscan->phs_relid); + Assert(RelFileLocatorEquals(relation->rd_locator, pscan->phs_locator)); if (!pscan->phs_snapshot_any) { @@ -389,7 +389,7 @@ table_block_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan) { ParallelBlockTableScanDesc bpscan = (ParallelBlockTableScanDesc) pscan; - bpscan->base.phs_relid = RelationGetRelid(rel); + bpscan->base.phs_locator = rel->rd_locator; bpscan->phs_nblocks = RelationGetNumberOfBlocks(rel); /* compare phs_syncscan initialization to similar logic in initscan */ bpscan->base.phs_syncscan = synchronize_seqscans && diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index 521043304a..114a85dc47 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -18,6 +18,7 @@ #include "access/itup.h" #include "port/atomics.h" #include "storage/buf.h" +#include "storage/relfilelocator.h" #include "storage/spin.h" #include "utils/relcache.h" @@ -62,7 +63,7 @@ typedef struct TableScanDescData *TableScanDesc; */ typedef struct ParallelTableScanDescData { - Oid phs_relid; /* OID of relation to scan */ + RelFileLocator phs_locator; /* physical relation to scan */ bool phs_syncscan; /* report location to syncscan logic? */ bool phs_snapshot_any; /* SnapshotAny, not phs_snapshot_data? */ Size phs_snapshot_off; /* data for snapshot */ @@ -169,8 +170,8 @@ typedef struct IndexScanDescData /* Generic structure for parallel scans */ typedef struct ParallelIndexScanDescData { - Oid ps_relid; - Oid ps_indexid; + RelFileLocator ps_locator; /* physical table relation to scan */ + RelFileLocator ps_indexlocator; /* physical index relation to scan */ Size ps_offset; /* Offset in bytes of am specific structure */ char ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER]; } ParallelIndexScanDescData; ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-09-19 23:45 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-11-20 02:48 [PATCH 1/3] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]> 2024-09-19 23:45 Rethinking parallel-scan relation identity checks Tom Lane <[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