RE: extension patch of CREATE OR REPLACE TRIGGER - Mailing list pgsql-hackers

From tsunakawa.takay@fujitsu.com
Subject RE: extension patch of CREATE OR REPLACE TRIGGER
Date
Msg-id TYAPR01MB2990041076AC194015D77A0DFE160@TYAPR01MB2990.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: extension patch of CREATE OR REPLACE TRIGGER  ("osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>)
Responses RE: extension patch of CREATE OR REPLACE TRIGGER  ("osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>)
List pgsql-hackers
From: osumi.takamichi@fujitsu.com <osumi.takamichi@fujitsu.com>
> > > * I don't think that you've fully thought through the implications
> > > of replacing a trigger for a table that the current transaction has
> > > already modified.  Is it really sufficient, or even useful, to do
> > > this:
> > >
> > > +            /*
> > > +             * If this trigger has pending events, throw an error.
> > > +             */
> > > +            if (trigger_deferrable &&
> > > + AfterTriggerPendingOnRel(RelationGetRelid(rel)))
> > >
> > > As an example, if we change a BEFORE trigger to an AFTER trigger,
> > > that's not going to affect the fact that we *already* fired that
> > > trigger.  Maybe this is okay and we just need to document it, but
> > > I'm not
> > convinced.
> > >
> > > * BTW, I don't think a trigger necessarily has to be deferrable in
> > > order to have pending AFTER events.  The existing use of
> > > AfterTriggerPendingOnRel certainly doesn't assume that.  But really,
> > > I think we probably ought to be applying CheckTableNotInUse which'd
> > > include that test.  (Another way in which there's fuzzy thinking
> > > here is that AfterTriggerPendingOnRel isn't specific to *this*
> > > trigger.)
> > Hmm, actually, when I just put a code of CheckTableNotInUse() in
> > CreateTrigger(), it throws error like "cannot CREATE OR REPLACE
> > TRIGGER because it is being used by active queries in this session".
> > This causes an break of the protection for internal cases above and a
> > contradiction of already passed test cases.
> > I though adding a condition of in_partition==false to call
> CheckTableNotInUse().
> > But this doesn't work in a corner case that child trigger generated
> > internally is pending and we don't want to allow the replacement of this kind
> of trigger.
> > Did you have any good idea to achieve both points at the same time ?
> Still, in terms of this point, I'm waiting for a comment !

I understand this patch is intended for helping users to migrate from other DBMSs (mainly Oracle?) because they can
easilyalter some trigger attributes (probably the trigger action and WHEN condition in practice.)  OTOH, the above
issueseems to be associated with the Postgres-specific constraint trigger that is created with CONSTRAINT clause.
(Oracleand the SQL standard doesn't have an equivalent feature.) 

So, how about just disallowing the combination of REPLACE and CONSTRAINT?  I think nobody would be crippled with that.
Ifsomeone wants the combination by all means, that can be a separate enhancement. 


Regards
Takayuki Tsunakawa




pgsql-hackers by date:

Previous
From: Amul Sul
Date:
Subject: Re: Assertion failure when ATTACH partition followed by CREATE PARTITION.
Next
From: Amit Langote
Date:
Subject: Re: Parallel Append can break run-time partition pruning