Re: As proposed the complete changes to pg_trigger and pg_rewrite

For discussion:

Attached is the completed patch that changes pg_trigger and extends
pg_rewrite in order to allow triggers and rules to be defined with
different, per session controllable, behaviors for replication purposes.

This will allow replication systems like Slony-I and, as has been stated
on pgsql-hackers, other products to control the firing mechanism of
triggers and rewrite rules without modifying the system catalog directly.

The firing mechanisms are controlled by a new superuser-only GUC
variable, session_replication_role, together with a change to
pg_trigger.tgenabled and a new column pg_rewrite.ev_enabled. Both
columns are a single char data type now (tgenabled was a bool before).
The possible values in these attributes are:

'O' - Trigger/Rule fires when session_replication_role is "origin"
(default) or "local". This is the default behavior.

'D' - Trigger/Rule is disabled and fires never

'A' - Trigger/Rule fires always regardless of the setting of
session_replication_role

'R' - Trigger/Rule fires when session_replication_role is "replica"

The GUC variable can only be changed as long as the system does not have
any saved SPI plans. This will prevent changing the session role and
accidentally executing stored procedures or functions that have plans
cached that expand to the wrong query set due to differences in the rule
firing semantics.

The SQL syntax for changing a triggers/rules firing semantics is

ALTER TABLE <tabname> <when> TRIGGER|RULE <name>;

<when> ::= ENABLE | ENABLE ALWAYS | ENABLE REPLICA | DISABLE

psql's \d command as well as pg_dump are extended in a backward
compatible fashion.

Any volunteers to do the corresponding documentation changes should this
patch be accepted?

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
> Attached is the completed patch that changes pg_trigger and extends
> pg_rewrite in order to allow triggers and rules to be defined with
> different, per session controllable, behaviors for replication purposes.

The SPI_savedplans part of this is pretty bletcherous, and has been
overtaken by events anyway. I'd suggest testing whether plancache.c
has anything in its list.

regards, tom lane

On 3/15/2007 11:16 PM, Tom Lane wrote:
> Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
>> Attached is the completed patch that changes pg_trigger and extends
>> pg_rewrite in order to allow triggers and rules to be defined with
>> different, per session controllable, behaviors for replication purposes.
>
> The SPI_savedplans part of this is pretty bletcherous, and has been
> overtaken by events anyway. I'd suggest testing whether plancache.c
> has anything in its list.

Hehe,

after today's commit by you it also throws a cvs merge conflict exactly
there ...

After a quick look it seems that another little function in there,
checking if (cached_plans_list == NIL), would do exactly that.

Easy enough to adjust to that.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

On 3/16/2007 12:12 AM, Jan Wieck wrote:
> On 3/15/2007 11:16 PM, Tom Lane wrote:
>> The SPI_savedplans part of this is pretty bletcherous, and has been
>> overtaken by events anyway. I'd suggest testing whether plancache.c
>> has anything in its list.
>
> [...]
>
> After a quick look it seems that another little function in there,
> checking if (cached_plans_list == NIL), would do exactly that.

Changed patch is attached.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

On 3/16/2007 9:03 AM, Jan Wieck wrote:
> Changed patch is attached.

Since a patch that touches as many files as this one does tends to
suffer from bitrot quited fast (cvs update conflicts during merge), can
we please move fast on this one? I think we had a general consensus that
the functionality itself is useful for several projects like Gorda and
Slony-I. Attached now is a patch including documentation changes. I will
commit this unless there are serious concerns or objections by Monday night.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

Ah, so you wait for me to go on vacation to apply it! Well, I am back
now, buddy. ;-)

One thing that bothers me about the patch is that it seems you are
adding functionality that allows you to enable/disable trigger firing in
groups, which is fine, but you are hard-coding the use of that grouping
to be replication, e.g. "origin", "replica", etc.

Should these designations be more generic in case there are other uses
for enabling/disabling groups of triggers?

---------------------------------------------------------------------------

Jan Wieck wrote:
> For discussion:
>
> Attached is the completed patch that changes pg_trigger and extends
> pg_rewrite in order to allow triggers and rules to be defined with
> different, per session controllable, behaviors for replication purposes.
>
> This will allow replication systems like Slony-I and, as has been stated
> on pgsql-hackers, other products to control the firing mechanism of
> triggers and rewrite rules without modifying the system catalog directly.
>
> The firing mechanisms are controlled by a new superuser-only GUC
> variable, session_replication_role, together with a change to
> pg_trigger.tgenabled and a new column pg_rewrite.ev_enabled. Both
> columns are a single char data type now (tgenabled was a bool before).
> The possible values in these attributes are:
>
> 'O' - Trigger/Rule fires when session_replication_role is "origin"
> (default) or "local". This is the default behavior.
>
> 'D' - Trigger/Rule is disabled and fires never
>
> 'A' - Trigger/Rule fires always regardless of the setting of
> session_replication_role
>
> 'R' - Trigger/Rule fires when session_replication_role is "replica"
>
> The GUC variable can only be changed as long as the system does not have
> any saved SPI plans. This will prevent changing the session role and
> accidentally executing stored procedures or functions that have plans
> cached that expand to the wrong query set due to differences in the rule
> firing semantics.
>
> The SQL syntax for changing a triggers/rules firing semantics is
>
> ALTER TABLE <tabname> <when> TRIGGER|RULE <name>;
>
> <when> ::= ENABLE | ENABLE ALWAYS | ENABLE REPLICA | DISABLE
>
> psql's \d command as well as pg_dump are extended in a backward
> compatible fashion.
>
> Any volunteers to do the corresponding documentation changes should this
> patch be accepted?
>

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On 3/21/2007 10:24 PM, Bruce Momjian wrote:
> Ah, so you wait for me to go on vacation to apply it! Well, I am back
> now, buddy. ;-)

Got to use my chances, don't I? :-p

>
> One thing that bothers me about the patch is that it seems you are
> adding functionality that allows you to enable/disable trigger firing in
> groups, which is fine, but you are hard-coding the use of that grouping
> to be replication, e.g. "origin", "replica", etc.
>
> Should these designations be more generic in case there are other uses
> for enabling/disabling groups of triggers?

That would be fine with me, I just wasn't able to come up with any
sensible naming scheme other than replication related. Can you?

Jan

>
> ---------------------------------------------------------------------------
>
> Jan Wieck wrote:
>> For discussion:
>>
>> Attached is the completed patch that changes pg_trigger and extends
>> pg_rewrite in order to allow triggers and rules to be defined with
>> different, per session controllable, behaviors for replication purposes.
>>
>> This will allow replication systems like Slony-I and, as has been stated
>> on pgsql-hackers, other products to control the firing mechanism of
>> triggers and rewrite rules without modifying the system catalog directly.
>>
>> The firing mechanisms are controlled by a new superuser-only GUC
>> variable, session_replication_role, together with a change to
>> pg_trigger.tgenabled and a new column pg_rewrite.ev_enabled. Both
>> columns are a single char data type now (tgenabled was a bool before).
>> The possible values in these attributes are:
>>
>> 'O' - Trigger/Rule fires when session_replication_role is "origin"
>> (default) or "local". This is the default behavior.
>>
>> 'D' - Trigger/Rule is disabled and fires never
>>
>> 'A' - Trigger/Rule fires always regardless of the setting of
>> session_replication_role
>>
>> 'R' - Trigger/Rule fires when session_replication_role is "replica"
>>
>> The GUC variable can only be changed as long as the system does not have
>> any saved SPI plans. This will prevent changing the session role and
>> accidentally executing stored procedures or functions that have plans
>> cached that expand to the wrong query set due to differences in the rule
>> firing semantics.
>>
>> The SQL syntax for changing a triggers/rules firing semantics is
>>
>> ALTER TABLE <tabname> <when> TRIGGER|RULE <name>;
>>
>> <when> ::= ENABLE | ENABLE ALWAYS | ENABLE REPLICA | DISABLE
>>
>> psql's \d command as well as pg_dump are extended in a backward
>> compatible fashion.
>>
>> Any volunteers to do the corresponding documentation changes should this
>> patch be accepted?
>>
>

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

Jan Wieck wrote:
> On 3/21/2007 10:24 PM, Bruce Momjian wrote:
> > Ah, so you wait for me to go on vacation to apply it! Well, I am back
> > now, buddy. ;-)
>
> Got to use my chances, don't I? :-p
>
> >
> > One thing that bothers me about the patch is that it seems you are
> > adding functionality that allows you to enable/disable trigger firing in
> > groups, which is fine, but you are hard-coding the use of that grouping
> > to be replication, e.g. "origin", "replica", etc.
> >
> > Should these designations be more generic in case there are other uses
> > for enabling/disabling groups of triggers?
>
> That would be fine with me, I just wasn't able to come up with any
> sensible naming scheme other than replication related. Can you?

The best I could think of would be to create numbered groups of
triggers, but because I can't think of any use for that, and no one else
has, I think the patch is fine unchanged.

---------------------------------------------------------------------------

> > Jan Wieck wrote:
> >> For discussion:
> >>
> >> Attached is the completed patch that changes pg_trigger and extends
> >> pg_rewrite in order to allow triggers and rules to be defined with
> >> different, per session controllable, behaviors for replication purposes.
> >>
> >> This will allow replication systems like Slony-I and, as has been stated
> >> on pgsql-hackers, other products to control the firing mechanism of
> >> triggers and rewrite rules without modifying the system catalog directly.
> >>
> >> The firing mechanisms are controlled by a new superuser-only GUC
> >> variable, session_replication_role, together with a change to
> >> pg_trigger.tgenabled and a new column pg_rewrite.ev_enabled. Both
> >> columns are a single char data type now (tgenabled was a bool before).
> >> The possible values in these attributes are:
> >>
> >> 'O' - Trigger/Rule fires when session_replication_role is "origin"
> >> (default) or "local". This is the default behavior.
> >>
> >> 'D' - Trigger/Rule is disabled and fires never
> >>
> >> 'A' - Trigger/Rule fires always regardless of the setting of
> >> session_replication_role
> >>
> >> 'R' - Trigger/Rule fires when session_replication_role is "replica"
> >>
> >> The GUC variable can only be changed as long as the system does not have
> >> any saved SPI plans. This will prevent changing the session role and
> >> accidentally executing stored procedures or functions that have plans
> >> cached that expand to the wrong query set due to differences in the rule
> >> firing semantics.
> >>
> >> The SQL syntax for changing a triggers/rules firing semantics is
> >>
> >> ALTER TABLE <tabname> <when> TRIGGER|RULE <name>;
> >>
> >> <when> ::= ENABLE | ENABLE ALWAYS | ENABLE REPLICA | DISABLE
> >>
> >> psql's \d command as well as pg_dump are extended in a backward
> >> compatible fashion.
> >>
> >> Any volunteers to do the corresponding documentation changes should this
> >> patch be accepted?
> >>
> >
>
>
> --
> #======================================================================#
> # It's easier to get forgiveness for being wrong than for being right. #
> # Let's break this rule - forgive me. #
> #================================================== JanWieck(at)Yahoo(dot)com #

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

>>>
>>> Should these designations be more generic in case there are other uses
>>> for enabling/disabling groups of triggers?
>> That would be fine with me, I just wasn't able to come up with any
>> sensible naming scheme other than replication related. Can you?
>
> The best I could think of would be to create numbered groups of
> triggers, but because I can't think of any use for that, and no one else
> has, I think the patch is fine unchanged.

Instead of numbering, why don't you use names ?

------------------------------------------------------------------------

I did a similar patch a couple of years ago, but I put it away
as it required modifications to the database catalog.
I am glad to hear that modifications to make replication easier on top
of PostgreSQL are being made.

Do you have any plans in introducing flags for other types of
constraints and even sequences. Such modifications would be very useful
for replication.

Best regards,

Alfranio.