Dear Tom Lane Thank you so much for your comment.
> * Upthread you asked about changing the lock level to be AccessExclusiveLock
> if
> the trigger already exists, but the patch doesn't actually do that. Which is
> fine by
> me, because that sounds like a perfectly bad idea.
Why I suggested a discussion
to make the lock level of C.O.R.T. stronger above comes from my concern.
I've worried about a case that
C.O.R.T. weak lock like ShareRowExclusiveLock allows
one session to replace other session's trigger for new trigger by COMMIT;
As a result, the session is made to use the new one unintentionally.
As you can see below, the previous trigger is replaced by Session2 after
applying this patch.
This seems to conflict with user's expectation to data consistency between
sessions or
to identify C.O.R.T with DROP TRIGGER (AcessExclusive) + CREATE TRIGGER in
terms of lock level.
-- Preparation
create table my_table1 (id integer, name text);
create table my_table2 (id integer, name text);
CREATE OR REPLACE FUNCTION public.my_updateproc1() RETURNS trigger LANGUAGE
plpgsql
AS $function$
begin
UPDATE my_table2 SET name = 'new ' WHERE id=OLD.id;
RETURN NULL;
end;$function$;
CREATE OR REPLACE FUNCTION public.my_updateproc2() RETURNS trigger LANGUAGE
plpgsql
AS $function$
begin
UPDATE my_table2 SET name = 'replace' WHERE id=OLD.id;
RETURN NULL;
end;$function$;
CREATE OR REPLACE TRIGGER my_regular_trigger AFTER UPDATE
ON my_table1 FOR EACH ROW EXECUTE PROCEDURE my_updateproc1();
--Session 1---
BEGIN;
select * from my_table1; -- Cause AccessShareLock here by referring to
my_table1;
--Session 2---
BEGIN;
CREATE OR REPLACE TRIGGER my_regular_trigger
AFTER UPDATE ON my_table1 FOR EACH ROW
EXECUTE PROCEDURE my_updateproc2();
COMMIT;
--Session 1---
select pg_get_triggerdef(oid, true) from pg_trigger where tgrelid =
'my_table1'::regclass AND tgname = 'my_regular_trigger';
------------------------------------------------------------------------------------------------------------
CREATE TRIGGER my_regular_trigger AFTER UPDATE ON my_table1 FOR EACH ROW
EXECUTE FUNCTION my_updateproc2()
(1 row)
By the way, I've fixed other points of my previous patch.
> * I wouldn't recommend adding CreateConstraintEntry's new argument at the end.
I changed the order of CreateConstraintEntry function and its header comment.
Besides that,
> I'm not on board with the idea that testing trigger_exists three separate
> times, in
> three randomly-different-looking ways, makes things more readable.
I did code refactoring of the redundant and confusing part.
> We do not test \h output in any existing regression test
And off course, I deleted the \h test you mentioned above.
Regards,
Takamichi Osumi
CREATE_OR_REPLACE_TRIGGER_v04.patch
Description: CREATE_OR_REPLACE_TRIGGER_v04.patch
