Обсуждение: BUG #14431: ERROR: XX000: relation 5022917 has no triggers ("should not happen")

VGhlIGZvbGxvd2luZyBidWcgaGFzIGJlZW4gbG9nZ2VkIG9uIHRoZSB3ZWJz
aXRlOgoKQnVnIHJlZmVyZW5jZTogICAgICAxNDQzMQpMb2dnZWQgYnk6ICAg
ICAgICAgIEJlbmppZSBHaWxsYW0KRW1haWwgYWRkcmVzczogICAgICBiZW5q
aWVAamVtamllLmNvbQpQb3N0Z3JlU1FMIHZlcnNpb246IDkuNS4yCk9wZXJh
dGluZyBzeXN0ZW06ICAgT1MgWCAxMC4xMS41CkRlc2NyaXB0aW9uOiAgICAg
ICAgCgpJJ3ZlIHRyaWdnZXJlZCBhbiBlcnJvciB0aGF0ICJzaG91bGQgbm90
IGhhcHBlbiIsIHRvb2sgbWUgYSB3aGlsZSB0byBmaWd1cmUKb3V0IHdoYXQg
SSdkIGRvbmUuDQoNCkkndmUgbWFuYWdlZCB0byBidWlsZCBhIHNtYWxsIHRl
c3QgY2FzZSB0aGF0IHJlcHJvZHVjZXMgdGhlIGlzc3VlIChpdApyZWxhdGVz
IHRvIGBkZWZlcnJhYmxlIGluaXRpYWxseSBkZWZlcnJlZGApOg0KDQpodHRw
czovL2dpc3QuZ2l0aHViLmNvbS9iZW5qaWUvOWQ0ZjhkZjQxZWJmN2EwZmI3
ODc4OGMwMGM3NTU2MWENCg0KSSBkb24ndCB0aGluayBpdCdzIGEgcGFydGlj
dWxhcmx5IGltcG9ydGFudCBpc3N1ZS4gSGVyZSdzIHRoZSByZWxldmFudCBs
aW5lCm9mIGNvZGU6DQoNCmh0dHBzOi8vZ2l0aHViLmNvbS9wb3N0Z3Jlcy9w
b3N0Z3Jlcy9ibG9iLzZmYTM5MWJlNGU4MzEzOWNkMTM0ZDVjY2ZjMTQ5OTgw
OWJiOGM5OGMvc3JjL2JhY2tlbmQvY29tbWFuZHMvdHJpZ2dlci5jI0w0MDI3
LUw0MDI5Cgo=

On 11/24/2016 12:49 PM, benjie@jemjie.com wrote:
> The following bug has been logged on the website:
>
> Bug reference:      14431
> Logged by:          Benjie Gillam
> Email address:      benjie@jemjie.com
> PostgreSQL version: 9.5.2
> Operating system:   OS X 10.11.5
> Description:
>
> I've triggered an error that "should not happen", took me a while to figure
> out what I'd done.
>
> I've managed to build a small test case that reproduces the issue (it
> relates to `deferrable initially deferred`):
>
> https://gist.github.com/benjie/9d4f8df41ebf7a0fb78788c00c75561a

For the archives, the test case at that link is:

<>
begin;
  create table a(foo varchar not null primary key);
  create table b(foo varchar not null primary key, constraint qux
foreign key (foo) references a deferrable initially deferred);
  create function bar() returns trigger as $$
  begin
    if TG_OP = 'INSERT' then
      insert into b(foo) values(new.foo);
    elsif TG_OP = 'DELETE' then
      delete from b where foo = old.foo;
    end if;
    return new;
  end;
  $$ language plpgsql;

  create trigger bar after insert or delete on a for each row execute
procedure bar();
commit;

insert into a values('baz');

begin;
  delete from a where foo = 'baz';
  drop trigger bar on a;
  drop function bar();
  alter table b drop constraint qux;
commit;
</>

I can reproduce it.

> I don't think it's a particularly important issue. Here's the relevant line
> of code:
>
>
https://github.com/postgres/postgres/blob/6fa391be4e83139cd134d5ccfc1499809bb8c98c/src/backend/commands/trigger.c#L4027-L4029


--
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Vik Fearing <vik@2ndquadrant.fr> writes:
> For the archives, the test case at that link is:

Thanks for copying that into the archives.

The "bar" trigger is actually irrelevant and can be dropped from the
example.  The problem is that because the FK constraint on "b" is
deferred, there is an FK-check trigger event waiting to be fired
at the end of the transaction, and we're deleting the pg_trigger
row out from under it.

Ordinarily, we'd prevent you from dropping a trigger that has
unfired events, but the code in tablecmds.c is only checking the
"b" table, and the event in question is for "a"'s trigger.

With the attached patch, you get

ERROR:  cannot ALTER TABLE "a" because it has pending trigger events

which I think is a sane behavior, although maybe the precise wording
of the error could be quibbled with.  (But improving that would
require fooling with the API of CheckTableNotInUse, which does not
seem worthwhile.)

The other approach we could take is to allow the DROP CONSTRAINT and
silently ignore queued trigger events if we can't find their pg_trigger
row ... but that seems pretty scary in terms of its ability to mask bugs,
and it's not consistent with past choices.

Barring objection, I'll add a test case to this and push it.

            regards, tom lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f97bee5..6322fa7 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** ATExecDropConstraint(Relation rel, const
*** 7723,7728 ****
--- 7723,7746 ----
          is_no_inherit_constraint = con->connoinherit;

          /*
+          * If it's a foreign-key constraint, we'd better lock the referenced
+          * table and check that that's not in use, just as we've already done
+          * for the constrained table (else we might, eg, be dropping a trigger
+          * that has unfired events).  But we can/must skip that in the
+          * self-referential case.
+          */
+         if (con->contype == CONSTRAINT_FOREIGN &&
+             con->confrelid != RelationGetRelid(rel))
+         {
+             Relation    frel;
+
+             /* Must match lock taken by RemoveTriggerById: */
+             frel = heap_open(con->confrelid, AccessExclusiveLock);
+             CheckTableNotInUse(frel, "ALTER TABLE");
+             heap_close(frel, NoLock);
+         }
+
+         /*
           * Perform the actual constraint deletion
           */
          conobj.classId = ConstraintRelationId;