Hello
On Friday, November 6, 2020 2:25 PM Peter Smith <smithpb2...@gmail.com> wrote:
> > > Yes, OK. But it might be simpler still just to it like:
> > > "CREATE OR REPLACE TRIGGER works only for replacing a regular (not
> > > constraint) trigger with another regular trigger."
> > Yeah, this kind of supplementary words help user to understand the
> > exact usage of this feature. Thanks.
>
> Actually, I meant that after making that 1st sentence wording change, I
> thought the 2nd sentence (i.e. "That means it is impossible...") is no longer
> needed at all since it is just re-stating what the 1st sentence already says.
>
> But if you prefer to leave it worded how it is now that is ok too.
The simpler, the better for sure ? I deleted that 2nd sentence.
> > > > > (9) COMMENT
> (snip)
> > I understand that
> > I need to add 2 syntax error cases and
> > 1 error case to replace constraint trigger at least. It makes sense.
> > At the same time, I supposed that the order of the tests in v15 patch
> > is somehow hard to read.
> > So, I decided to sort out those and take your new sets of tests there.
> > What I'd like to test there is not different, though.
> > Please have a look at the new patch.
>
> Yes, the tests are generally OK, but unfortunately a few new problems are
> introduced with the refactoring of the combination tests.
>
> 1) It looks like about 40 lines of test code are cut/paste 2 times by accident
This was not a mistake. The cases of 40 lines are with OR REPLACE to define
each regular trigger that will be overwritten.
But, it doesn't make nothing probably so I deleted such cases.
Please forget that part.
> 2) Typo "gramatically" --> "grammatically"
> 3) Your last test described as "create or replace constraint trigger is not
> gramatically correct." is not really doing what it is meant to do. That test
> was
> supposed to be trying to replace an existing CONSTRAINT trigger.
Sigh. Yeah, those were not right. Fixed.
>
> IMO if all the combination tests were consistently commented like my 8
> examples below then risk of accidental mistakes is reduced.
> e.g.
> -- 1. Overwrite existing regular trigger with regular trigger (without OR
> REPLACE)
> -- 2. Overwrite existing regular trigger with regular trigger (with OR
> REPLACE)
> -- 3. Overwrite existing regular trigger with constraint trigger (without OR
> REPLACE)
> -- 4. Overwrite existing regular trigger with constraint trigger (with OR
> REPLACE)
> -- 5. Overwrite existing constraint trigger with regular trigger (without OR
> REPLACE)
> -- 6. Overwrite existing constraint trigger with regular trigger (with OR
> REPLACE)
> -- 7. Overwrite existing constraint trigger with constraint trigger (without
> OR
> REPLACE)
> -- 8. Overwrite existing constraint trigger with constraint trigger (with OR
> REPLACE)
>
> To avoid any confusion I have attached triggers.sql updated how I think it
> should be. Please compare it to see what I mean. PSA.
>
> I hope it helps.
I cannot thank you enough.
Best,
Takamichi Osumi
CREATE_OR_REPLACE_TRIGGER_v17.patch
Description: CREATE_OR_REPLACE_TRIGGER_v17.patch
