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

From osumi.takamichi@fujitsu.com
Subject RE: extension patch of CREATE OR REPLACE TRIGGER
Date
Msg-id OSBPR01MB48888E29DD98F5F0CDA47DB2ED260@OSBPR01MB4888.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: extension patch of CREATE OR REPLACE TRIGGER  (Peter Smith <smithpb2250@gmail.com>)
Responses Re: extension patch of CREATE OR REPLACE TRIGGER  (Peter Smith <smithpb2250@gmail.com>)
List pgsql-hackers
Hi


> Those are fixed OK now, but I found 2 new review points.
> 
> ====
> 
> COMMENT trigger.c (typo)
> 
> + ereport(ERROR,
> + (errcode(ERRCODE_DUPLICATE_OBJECT),
> + errmsg("trigger \"%s\" for relation \"%s\" is a constraint trigger",
> + stmt->trigname, RelationGetRelationName(rel)),
> + errhint("use CREATE OR REPLACE CONSTRAINT TRIGGER to replace a
> costraint trigger")));
> 
> 
> Typo in the errhint text.
> "costraint" -> "constraint"
Fixed. Thank you.

> ====
> 
> COMMENT create_trigger.sgmg (add more help?)
> 
> I noticed that the CREATE OR REPLACE FUNCTION help [1] describes the OR
> REPLACE syntax ("Description" section) and also mentions some of the
> restrictions when using REPLACE ("Notes" section).
> [1] - https://www.postgresql.org/docs/current/sql-createfunction.html
> 
> ~~
> OTOH this trigger patch does not add anything much at all in the trigger help.
> 
> Shouldn't the "Description" at least say something like:
> "CREATE OR REPLACE will either create a new trigger, or replace an existing
> definition."
> 
> Shouldn't the "Notes" include information about restrictions when using OR
> REPLACE e.g. cannot replace triggers with triggers of a different kind e.g.
> cannot replace triggers with pending events
> 
> What do you think?
That's a great idea. I've applied this idea to the latest patch v10.

Regards,
    Takamichi Osumi

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Fix for parallel BTree initialization bug
Next
From: Etsuro Fujita
Date:
Subject: Minor cleanup of partbounds.c