pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_trigger.tgparentid
10+ messages / 3 participants
[nested] [flat]
* pg_trigger.tgparentid
@ 2020-02-17 21:56 Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-17 21:59 ` Re: pg_trigger.tgparentid Tom Lane <tgl@sss.pgh.pa.us>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
0 siblings, 2 replies; 10+ messages in thread
From: Alvaro Herrera @ 2020-02-17 21:56 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Amit Langote <amitlangote09@gmail.com>
As mentioned in https://postgr.es/m/20191231194759.GA24692@alvherre.pgsql
I propose to add a new column to pg_trigger, which allows us to remove a
pg_depend scan when cloning triggers when adding/attaching partitions.
(It's not that I think the scan is a performance problem, but rather
than notionally we try not to depend on pg_depend contents for this kind
of semantic derivation.)
--
Álvaro Herrera 39°49'30"S 73°17'W
Attachments:
0001-Record-parents-of-triggers.patch (text/x-diff, 4.8K)
download | inline diff:
From 8f1be4b2c24385bbfe2ecba941545ffa84f60b47 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 17 Feb 2020 17:34:47 -0300
Subject: [PATCH] Record parents of triggers
This let us get rid of a recently introduced ugly hack (commit
1fa846f1c9af).
---
doc/src/sgml/catalogs.sgml | 7 +++++
src/backend/commands/tablecmds.c | 52 ++------------------------------
src/backend/commands/trigger.c | 1 +
src/include/catalog/pg_trigger.h | 1 +
4 files changed, 11 insertions(+), 50 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index a10b66569b..0bba96ee52 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6951,6 +6951,13 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<entry>The table this trigger is on</entry>
</row>
+ <row>
+ <entry><structfield>tgparentid</structfield></entry>
+ <entry><type>oid</type></entry>
+ <entry><literal><link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link>.oid</literal></entry>
+ <entry>Parent trigger of this trigger, if any</entry>
+ </row>
+
<row>
<entry><structfield>tgname</structfield></entry>
<entry><type>name</type></entry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b7c8d663fc..ae39bf1056 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16447,54 +16447,6 @@ out:
MemoryContextDelete(cxt);
}
-/*
- * isPartitionTrigger
- * Subroutine for CloneRowTriggersToPartition: determine whether
- * the given trigger has been cloned from another one.
- *
- * We use pg_depend as a proxy for this, since we don't have any direct
- * evidence. This is an ugly hack to cope with a catalog deficiency.
- * Keep away from children. Do not stare with naked eyes. Do not propagate.
- */
-static bool
-isPartitionTrigger(Oid trigger_oid)
-{
- Relation pg_depend;
- ScanKeyData key[2];
- SysScanDesc scan;
- HeapTuple tup;
- bool found = false;
-
- pg_depend = table_open(DependRelationId, AccessShareLock);
-
- ScanKeyInit(&key[0], Anum_pg_depend_classid,
- BTEqualStrategyNumber,
- F_OIDEQ,
- ObjectIdGetDatum(TriggerRelationId));
- ScanKeyInit(&key[1], Anum_pg_depend_objid,
- BTEqualStrategyNumber,
- F_OIDEQ,
- ObjectIdGetDatum(trigger_oid));
-
- scan = systable_beginscan(pg_depend, DependDependerIndexId,
- true, NULL, 2, key);
- while ((tup = systable_getnext(scan)) != NULL)
- {
- Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(tup);
-
- if (dep->refclassid == TriggerRelationId)
- {
- found = true;
- break;
- }
- }
-
- systable_endscan(scan);
- table_close(pg_depend, AccessShareLock);
-
- return found;
-}
-
/*
* CloneRowTriggersToPartition
* subroutine for ATExecAttachPartition/DefineRelation to create row
@@ -16541,7 +16493,7 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
*
* However, if our parent is a partitioned relation, there might be
* internal triggers that need cloning. In that case, we must skip
- * clone it if the trigger on parent depends on another trigger.
+ * cloning it if the trigger on parent depends on another trigger.
*
* Note we dare not verify that the other trigger belongs to an
* ancestor relation of our parent, because that creates deadlock
@@ -16549,7 +16501,7 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
*/
if (trigForm->tgisinternal &&
(!parent->rd_rel->relispartition ||
- !isPartitionTrigger(trigForm->oid)))
+ !OidIsValid(trigForm->tgparentid)))
continue;
/*
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index b9b1262e30..6e8b7223fe 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -847,6 +847,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
values[Anum_pg_trigger_oid - 1] = ObjectIdGetDatum(trigoid);
values[Anum_pg_trigger_tgrelid - 1] = ObjectIdGetDatum(RelationGetRelid(rel));
+ values[Anum_pg_trigger_tgparentid - 1] = ObjectIdGetDatum(parentTriggerOid);
values[Anum_pg_trigger_tgname - 1] = DirectFunctionCall1(namein,
CStringGetDatum(trigname));
values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid);
diff --git a/src/include/catalog/pg_trigger.h b/src/include/catalog/pg_trigger.h
index 2f30c7c2f9..9612b9bdd6 100644
--- a/src/include/catalog/pg_trigger.h
+++ b/src/include/catalog/pg_trigger.h
@@ -35,6 +35,7 @@ CATALOG(pg_trigger,2620,TriggerRelationId)
{
Oid oid; /* oid */
Oid tgrelid; /* relation trigger is attached to */
+ Oid tgparentid; /* OID of parent trigger, if any */
NameData tgname; /* trigger's name */
Oid tgfoid; /* OID of function to be called */
int16 tgtype; /* BEFORE/AFTER/INSTEAD, UPDATE/DELETE/INSERT,
--
2.20.1
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-02-17 21:59 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 10+ messages in thread
From: Tom Lane @ 2020-02-17 21:59 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>; Amit Langote <amitlangote09@gmail.com>
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> As mentioned in https://postgr.es/m/20191231194759.GA24692@alvherre.pgsql
> I propose to add a new column to pg_trigger, which allows us to remove a
> pg_depend scan when cloning triggers when adding/attaching partitions.
> (It's not that I think the scan is a performance problem, but rather
> than notionally we try not to depend on pg_depend contents for this kind
> of semantic derivation.)
It'd be nice if the term "parent trigger" were defined somewhere in
this. Seems all right otherwise.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-02-18 04:11 ` Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
1 sibling, 1 reply; 10+ messages in thread
From: Amit Langote @ 2020-02-18 04:11 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Feb 18, 2020 at 6:56 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> As mentioned in https://postgr.es/m/20191231194759.GA24692@alvherre.pgsql
> I propose to add a new column to pg_trigger, which allows us to remove a
> pg_depend scan when cloning triggers when adding/attaching partitions.
> (It's not that I think the scan is a performance problem, but rather
> than notionally we try not to depend on pg_depend contents for this kind
> of semantic derivation.)
@@ -16541,7 +16493,7 @@ CloneRowTriggersToPartition(Relation parent,
Relation partition)
*
* However, if our parent is a partitioned relation, there might be
This is existing text, but should really be:
However, if our parent is a *partition* itself, there might be
(Sorry, I forgot to report this when the bug-fix went in couple months ago.)
* internal triggers that need cloning. In that case, we must skip
- * clone it if the trigger on parent depends on another trigger.
+ * cloning it if the trigger on parent depends on another trigger.
2nd sentence seems unclear to me. Does the following say what needs
to be said here:
* However, if our parent is a partition itself, there might be
* internal triggers that need cloning. For example, triggers on the
* parent that were in turn cloned from its own parent are marked
* internal, which too must be cloned to the partition.
Thanks,
Amit
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
@ 2020-02-19 02:52 ` Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 10+ messages in thread
From: Amit Langote @ 2020-02-19 02:52 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Feb 18, 2020 at 1:11 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Tue, Feb 18, 2020 at 6:56 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> @@ -16541,7 +16493,7 @@ CloneRowTriggersToPartition(Relation parent,
> Relation partition)
> *
> * However, if our parent is a partitioned relation, there might be
>
> This is existing text, but should really be:
>
> However, if our parent is a *partition* itself, there might be
>
> (Sorry, I forgot to report this when the bug-fix went in couple months ago.)
>
> * internal triggers that need cloning. In that case, we must skip
> - * clone it if the trigger on parent depends on another trigger.
> + * cloning it if the trigger on parent depends on another trigger.
>
> 2nd sentence seems unclear to me. Does the following say what needs
> to be said here:
>
> * However, if our parent is a partition itself, there might be
> * internal triggers that need cloning. For example, triggers on the
> * parent that were in turn cloned from its own parent are marked
> * internal, which too must be cloned to the partition.
Or:
* However, if our parent is a partition itself, there might be
* internal triggers that must not be skipped. For example, triggers
* on the parent that were in turn cloned from its own parent are
* marked internal, which must be cloned to the partition.
Thanks,
Amit
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
@ 2020-02-24 18:58 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 01:26 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Alvaro Herrera @ 2020-02-24 18:58 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2020-Feb-19, Amit Langote wrote:
> Or:
>
> * However, if our parent is a partition itself, there might be
> * internal triggers that must not be skipped. For example, triggers
> * on the parent that were in turn cloned from its own parent are
> * marked internal, which must be cloned to the partition.
Thanks for pointing this out -- I agree it needed rewording. I slightly
adjusted your text like this:
* Internal triggers require careful examination. Ideally, we don't
* clone them. However, if our parent is itself a partition, there
* might be internal triggers that must not be skipped; for example,
* triggers on our parent that are in turn clones from its parent (our
* grandparent) are marked internal, yet they are to be cloned.
Is this okay for you?
Tom Lane wrote:
> It'd be nice if the term "parent trigger" were defined somewhere in
> this. Seems all right otherwise.
Fair point. I propose to patch catalog.sgml like this
<entry>
Parent trigger that this trigger is cloned from, zero if not a clone;
this happens when partitions are created or attached to a partitioned
table.
</entry>
It's perhaps not great to have to explain the parentage concept in the
catalog docs, so I'm going to go over the other documentation pages
(trigger.sgml and ref/create_trigger.sgml) to see whether they need any
patching; it's possible that we neglected to update them properly when
the partitioning-related commits went it.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Record-parents-of-triggers.patch (text/x-diff, 5.3K)
download | inline diff:
From ff13d7004c9ee77ba411ace405224188aa0c353c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 17 Feb 2020 17:34:47 -0300
Subject: [PATCH v2] Record parents of triggers
This let us get rid of a recently introduced ugly hack (commit
1fa846f1c9af).
---
doc/src/sgml/catalogs.sgml | 11 ++++++
src/backend/commands/tablecmds.c | 59 +++-----------------------------
src/backend/commands/trigger.c | 1 +
src/include/catalog/pg_trigger.h | 1 +
4 files changed, 18 insertions(+), 54 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index a10b66569b..34bc0d0526 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6951,6 +6951,17 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<entry>The table this trigger is on</entry>
</row>
+ <row>
+ <entry><structfield>tgparentid</structfield></entry>
+ <entry><type>oid</type></entry>
+ <entry><literal><link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link>.oid</literal></entry>
+ <entry>
+ Parent trigger that this trigger is cloned from, zero if not a clone;
+ this happens when partitions are created or attached to a partitioned
+ table.
+ </entry>
+ </row>
+
<row>
<entry><structfield>tgname</structfield></entry>
<entry><type>name</type></entry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b7c8d663fc..02a7c04fdb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16447,54 +16447,6 @@ out:
MemoryContextDelete(cxt);
}
-/*
- * isPartitionTrigger
- * Subroutine for CloneRowTriggersToPartition: determine whether
- * the given trigger has been cloned from another one.
- *
- * We use pg_depend as a proxy for this, since we don't have any direct
- * evidence. This is an ugly hack to cope with a catalog deficiency.
- * Keep away from children. Do not stare with naked eyes. Do not propagate.
- */
-static bool
-isPartitionTrigger(Oid trigger_oid)
-{
- Relation pg_depend;
- ScanKeyData key[2];
- SysScanDesc scan;
- HeapTuple tup;
- bool found = false;
-
- pg_depend = table_open(DependRelationId, AccessShareLock);
-
- ScanKeyInit(&key[0], Anum_pg_depend_classid,
- BTEqualStrategyNumber,
- F_OIDEQ,
- ObjectIdGetDatum(TriggerRelationId));
- ScanKeyInit(&key[1], Anum_pg_depend_objid,
- BTEqualStrategyNumber,
- F_OIDEQ,
- ObjectIdGetDatum(trigger_oid));
-
- scan = systable_beginscan(pg_depend, DependDependerIndexId,
- true, NULL, 2, key);
- while ((tup = systable_getnext(scan)) != NULL)
- {
- Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(tup);
-
- if (dep->refclassid == TriggerRelationId)
- {
- found = true;
- break;
- }
- }
-
- systable_endscan(scan);
- table_close(pg_depend, AccessShareLock);
-
- return found;
-}
-
/*
* CloneRowTriggersToPartition
* subroutine for ATExecAttachPartition/DefineRelation to create row
@@ -16537,11 +16489,10 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
/*
* Internal triggers require careful examination. Ideally, we don't
- * clone them.
- *
- * However, if our parent is a partitioned relation, there might be
- * internal triggers that need cloning. In that case, we must skip
- * clone it if the trigger on parent depends on another trigger.
+ * clone them. However, if our parent is itself a partition, there
+ * might be internal triggers that must not be skipped; for example,
+ * triggers on our parent that are in turn clones from its parent (our
+ * grandparent) are marked internal, yet they are to be cloned.
*
* Note we dare not verify that the other trigger belongs to an
* ancestor relation of our parent, because that creates deadlock
@@ -16549,7 +16500,7 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
*/
if (trigForm->tgisinternal &&
(!parent->rd_rel->relispartition ||
- !isPartitionTrigger(trigForm->oid)))
+ !OidIsValid(trigForm->tgparentid)))
continue;
/*
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index b9b1262e30..6e8b7223fe 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -847,6 +847,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
values[Anum_pg_trigger_oid - 1] = ObjectIdGetDatum(trigoid);
values[Anum_pg_trigger_tgrelid - 1] = ObjectIdGetDatum(RelationGetRelid(rel));
+ values[Anum_pg_trigger_tgparentid - 1] = ObjectIdGetDatum(parentTriggerOid);
values[Anum_pg_trigger_tgname - 1] = DirectFunctionCall1(namein,
CStringGetDatum(trigname));
values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid);
diff --git a/src/include/catalog/pg_trigger.h b/src/include/catalog/pg_trigger.h
index 2f30c7c2f9..9612b9bdd6 100644
--- a/src/include/catalog/pg_trigger.h
+++ b/src/include/catalog/pg_trigger.h
@@ -35,6 +35,7 @@ CATALOG(pg_trigger,2620,TriggerRelationId)
{
Oid oid; /* oid */
Oid tgrelid; /* relation trigger is attached to */
+ Oid tgparentid; /* OID of parent trigger, if any */
NameData tgname; /* trigger's name */
Oid tgfoid; /* OID of function to be called */
int16 tgtype; /* BEFORE/AFTER/INSTEAD, UPDATE/DELETE/INSERT,
--
2.20.1
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-02-25 01:26 ` Amit Langote <amitlangote09@gmail.com>
2020-02-25 14:00 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 10+ messages in thread
From: Amit Langote @ 2020-02-25 01:26 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Hi Alvaro,
On Tue, Feb 25, 2020 at 3:58 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2020-Feb-19, Amit Langote wrote:
>
> > Or:
> >
> > * However, if our parent is a partition itself, there might be
> > * internal triggers that must not be skipped. For example, triggers
> > * on the parent that were in turn cloned from its own parent are
> > * marked internal, which must be cloned to the partition.
>
> Thanks for pointing this out -- I agree it needed rewording. I slightly
> adjusted your text like this:
>
> * Internal triggers require careful examination. Ideally, we don't
> * clone them. However, if our parent is itself a partition, there
> * might be internal triggers that must not be skipped; for example,
> * triggers on our parent that are in turn clones from its parent (our
> * grandparent) are marked internal, yet they are to be cloned.
>
> Is this okay for you?
Thanks. Your revised text looks good, except there is a typo:
in turn clones -> in turn cloned
Thanks,
Amit
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 01:26 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
@ 2020-02-25 14:00 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 14:57 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Alvaro Herrera @ 2020-02-25 14:00 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On 2020-Feb-25, Amit Langote wrote:
> On Tue, Feb 25, 2020 at 3:58 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > Thanks for pointing this out -- I agree it needed rewording. I slightly
> > adjusted your text like this:
> >
> > * Internal triggers require careful examination. Ideally, we don't
> > * clone them. However, if our parent is itself a partition, there
> > * might be internal triggers that must not be skipped; for example,
> > * triggers on our parent that are in turn clones from its parent (our
> > * grandparent) are marked internal, yet they are to be cloned.
> >
> > Is this okay for you?
>
> Thanks. Your revised text looks good, except there is a typo:
>
> in turn clones -> in turn cloned
Actually, that was on purpose ... (I also changed "were" to "are" to match.)
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 01:26 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-25 14:00 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-02-25 14:57 ` Amit Langote <amitlangote09@gmail.com>
2020-02-27 16:26 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 10+ messages in thread
From: Amit Langote @ 2020-02-25 14:57 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
On Tue, Feb 25, 2020 at 11:01 PM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> On 2020-Feb-25, Amit Langote wrote:
> > On Tue, Feb 25, 2020 at 3:58 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > > Thanks for pointing this out -- I agree it needed rewording. I slightly
> > > adjusted your text like this:
> > >
> > > * Internal triggers require careful examination. Ideally, we don't
> > > * clone them. However, if our parent is itself a partition, there
> > > * might be internal triggers that must not be skipped; for example,
> > > * triggers on our parent that are in turn clones from its parent (our
> > > * grandparent) are marked internal, yet they are to be cloned.
> > >
> > > Is this okay for you?
> >
> > Thanks. Your revised text looks good, except there is a typo:
> >
> > in turn clones -> in turn cloned
>
> Actually, that was on purpose ... (I also changed "were" to "are" to match.)
Ah, got it.
Thanks,
Amit
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 01:26 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-25 14:00 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 14:57 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
@ 2020-02-27 16:26 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-27 16:32 ` Re: pg_trigger.tgparentid Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 10+ messages in thread
From: Alvaro Herrera @ 2020-02-27 16:26 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>; Tom Lane <tgl@sss.pgh.pa.us>
Thanks both -- pushed. I also changed regress/sql/triggers to leave
tables around that have a non-zero tgparentid. This ensures that the
pg_upgrade test sees such objects, as well as findoidjoins.
I refrained from doing the findoidjoins dance itself, though; I got a
large number of false positives that I think are caused by some pg12-era
hacking.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_trigger.tgparentid
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-18 04:11 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 01:26 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-25 14:00 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 14:57 ` Re: pg_trigger.tgparentid Amit Langote <amitlangote09@gmail.com>
2020-02-27 16:26 ` Re: pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-02-27 16:32 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 10+ messages in thread
From: Tom Lane @ 2020-02-27 16:32 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; Pg Hackers <pgsql-hackers@lists.postgresql.org>
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Thanks both -- pushed. I also changed regress/sql/triggers to leave
> tables around that have a non-zero tgparentid. This ensures that the
> pg_upgrade test sees such objects, as well as findoidjoins.
> I refrained from doing the findoidjoins dance itself, though; I got a
> large number of false positives that I think are caused by some pg12-era
> hacking.
Generally I try to update findoidjoins once per release cycle, after
feature freeze. I don't think it's worth messing with it more often
than that. But thanks for making sure there'll be data for it to find.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2020-02-27 16:32 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-02-17 21:56 pg_trigger.tgparentid Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-17 21:59 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-02-18 04:11 ` Amit Langote <amitlangote09@gmail.com>
2020-02-19 02:52 ` Amit Langote <amitlangote09@gmail.com>
2020-02-24 18:58 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 01:26 ` Amit Langote <amitlangote09@gmail.com>
2020-02-25 14:00 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-25 14:57 ` Amit Langote <amitlangote09@gmail.com>
2020-02-27 16:26 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-27 16:32 ` Tom Lane <tgl@sss.pgh.pa.us>
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