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
