public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 1/3] bootstrap: convert Typ to a List* 4+ messages / 3 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; 4+ 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 --------------8FD28E9B65A94BB176003065 Content-Type: text/x-patch; charset=UTF-8; name="0002-Allow-composite-types-in-bootstrap-20210116.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Allow-composite-types-in-bootstrap-20210116.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: \d with triggers: more than one row returned by a subquery used as an expression @ 2022-01-18 00:08 Justin Pryzby <[email protected]> 2022-01-18 00:14 ` Re: \d with triggers: more than one row returned by a subquery used as an expression Tom Lane <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Justin Pryzby @ 2022-01-18 00:08 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers; Amit Langote <[email protected]> On Mon, Jan 17, 2022 at 05:02:00PM -0500, Tom Lane wrote: > Justin Pryzby <[email protected]> writes: > > On Fri, Dec 17, 2021 at 09:43:56AM -0600, Justin Pryzby wrote: > >> I want to mention that the 2nd problem I mentioned here is still broken. > >> https://www.postgresql.org/message-id/[email protected] > >> It happens if non-inheritted triggers on child and parent have the same name. > > > This is the fix I was proposing > > It depends on pg_partition_ancestors() to return its partitions in order: > > this partition => parent => ... => root. > > I don't think that works at all. I might be willing to accept the > assumption about pg_partition_ancestors()'s behavior, but you're also > making an assumption about how the output of pg_partition_ancestors() > is joined to "pg_trigger AS u", and I really don't think that's safe. > ISTM the real problem is the assumption that only related triggers could > share a tgname, which evidently isn't true. I think this query needs to > actually match on tgparentid, rather than taking shortcuts. I don't think that should be needed - tgparentid should match pg_partition_ancestors(). > If we don't > want to use a recursive CTE, maybe we could define it as only looking up to > the immediate parent, rather than necessarily finding the root? I think that defeats the intent of c33869cc3. Is there any reason why WITH ORDINALITY can't work ? This is passing the smoke test. -- Justin Attachments: [text/x-diff] 0001-psql-Add-newlines-and-spacing-in-trigger-query-for-d.patch (1.5K, ../../[email protected]/2-0001-psql-Add-newlines-and-spacing-in-trigger-query-for-d.patch) download | inline diff: From 99f8ee921258d9b2e75299e69826004fda3b8919 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <[email protected]> Date: Fri, 3 Apr 2020 22:43:26 -0500 Subject: [PATCH 1/2] psql: Add newlines and spacing in trigger query for \d cosmetic change to c33869cc3 to make the output of psql -E look pretty. --- src/bin/psql/describe.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 61cec118aca..ed9f320b015 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2993,12 +2993,12 @@ describeOneTableDetails(const char *schemaname, "t.tgenabled, t.tgisinternal, %s\n" "FROM pg_catalog.pg_trigger t\n" "WHERE t.tgrelid = '%s' AND ", - (pset.sversion >= 130000 ? - "(SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass" - " FROM pg_catalog.pg_trigger AS u, " - " pg_catalog.pg_partition_ancestors(t.tgrelid) AS a" - " WHERE u.tgname = t.tgname AND u.tgrelid = a.relid" - " AND u.tgparentid = 0) AS parent" : + (pset.sversion >= 130000 ? "\n" + " (SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass\n" + " FROM pg_catalog.pg_trigger AS u,\n" + " pg_catalog.pg_partition_ancestors(t.tgrelid) AS a\n" + " WHERE u.tgname = t.tgname AND u.tgrelid = a.relid\n" + " AND u.tgparentid = 0) AS parent" : "NULL AS parent"), oid); -- 2.17.1 [text/x-diff] 0002-psql-fix-d-for-statement-triggers-with-same-name-on-.patch (3.2K, ../../[email protected]/3-0002-psql-fix-d-for-statement-triggers-with-same-name-on-.patch) download | inline diff: From d866624abbd523afdbc3a539f14c935c42fc345f Mon Sep 17 00:00:00 2001 From: Justin Pryzby <[email protected]> Date: Fri, 16 Jul 2021 19:57:00 -0500 Subject: [PATCH 2/2] psql: fix \d for statement triggers with same name on partition and its parent This depends on pg_partition_ancestors() to return its partitions in order: this partition => parent => ... => root. [email protected] --- src/bin/psql/describe.c | 4 ++-- src/test/regress/expected/triggers.out | 13 +++++++++++++ src/test/regress/sql/triggers.sql | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index ed9f320b015..8787849e8be 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2996,9 +2996,9 @@ describeOneTableDetails(const char *schemaname, (pset.sversion >= 130000 ? "\n" " (SELECT (NULLIF(a.relid, t.tgrelid))::pg_catalog.regclass\n" " FROM pg_catalog.pg_trigger AS u,\n" - " pg_catalog.pg_partition_ancestors(t.tgrelid) AS a\n" + " pg_catalog.pg_partition_ancestors(t.tgrelid) WITH ORDINALITY AS a(relid,depth)\n" " WHERE u.tgname = t.tgname AND u.tgrelid = a.relid\n" - " AND u.tgparentid = 0) AS parent" : + " AND u.tgparentid = 0 ORDER BY depth LIMIT 1) AS parent" : "NULL AS parent"), oid); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 5c0e7c2b79e..9278cc07237 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2122,6 +2122,19 @@ Triggers: alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail ERROR: trigger "trg1" for relation "trigpart3" already exists drop table trigpart3; +create trigger samename after delete on trigpart execute function trigger_nothing(); +create trigger samename after delete on trigpart1 execute function trigger_nothing(); +\d trigpart1 + Table "public.trigpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Partition of: trigpart FOR VALUES FROM (0) TO (1000) +Triggers: + samename AFTER DELETE ON trigpart1 FOR EACH STATEMENT EXECUTE FUNCTION trigger_nothing() + trg1 AFTER INSERT ON trigpart1 FOR EACH ROW EXECUTE FUNCTION trigger_nothing(), ON TABLE trigpart + drop table trigpart; drop function trigger_nothing(); -- diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 9cb15c21dc3..5e2197e10e2 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1428,6 +1428,10 @@ create trigger trg1 after insert on trigpart3 for each row execute procedure tri alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail drop table trigpart3; +create trigger samename after delete on trigpart execute function trigger_nothing(); +create trigger samename after delete on trigpart1 execute function trigger_nothing(); +\d trigpart1 + drop table trigpart; drop function trigger_nothing(); -- 2.17.1 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: \d with triggers: more than one row returned by a subquery used as an expression 2022-01-18 00:08 Re: \d with triggers: more than one row returned by a subquery used as an expression Justin Pryzby <[email protected]> @ 2022-01-18 00:14 ` Tom Lane <[email protected]> 2022-01-18 00:50 ` Re: \d with triggers: more than one row returned by a subquery used as an expression Tom Lane <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Tom Lane @ 2022-01-18 00:14 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers; Amit Langote <[email protected]> Justin Pryzby <[email protected]> writes: > On Mon, Jan 17, 2022 at 05:02:00PM -0500, Tom Lane wrote: >> ISTM the real problem is the assumption that only related triggers could >> share a tgname, which evidently isn't true. I think this query needs to >> actually match on tgparentid, rather than taking shortcuts. > I don't think that should be needed - tgparentid should match > pg_partition_ancestors(). Uh, what? tgparentid is a trigger OID, not a relation OID. > Is there any reason why WITH ORDINALITY can't work ? > This is passing the smoke test. How hard did you try to break it? It still seems to me that this can be fooled by an unrelated trigger with the same tgname. regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: \d with triggers: more than one row returned by a subquery used as an expression 2022-01-18 00:08 Re: \d with triggers: more than one row returned by a subquery used as an expression Justin Pryzby <[email protected]> 2022-01-18 00:14 ` Re: \d with triggers: more than one row returned by a subquery used as an expression Tom Lane <[email protected]> @ 2022-01-18 00:50 ` Tom Lane <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2022-01-18 00:50 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers; Amit Langote <[email protected]> I wrote: > Justin Pryzby <[email protected]> writes: >> Is there any reason why WITH ORDINALITY can't work ? >> This is passing the smoke test. > How hard did you try to break it? It still seems to me that > this can be fooled by an unrelated trigger with the same tgname. Hmm ... no, it does work, because we'll stop at the first trigger with tgparentid = 0, so unrelated triggers further up the partition stack don't matter. But this definitely requires commentary. (And I'm not too happy with burying such a complicated thing inside a conditional inside a printf, either.) Will see about cleaning it up. regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2022-01-18 00:50 UTC | newest] Thread overview: 4+ 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]> 2022-01-18 00:08 Re: \d with triggers: more than one row returned by a subquery used as an expression Justin Pryzby <[email protected]> 2022-01-18 00:14 ` Re: \d with triggers: more than one row returned by a subquery used as an expression Tom Lane <[email protected]> 2022-01-18 00:50 ` Re: \d with triggers: more than one row returned by a subquery used as an expression 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