Ticket #332 (closed bug: fixed)

Opened 22 months ago

Last modified 21 months ago

SQL for TRIGGER with WHEN clause broken

Reported by: brsa Owned by: gleu
Priority: minor Milestone: 1.14
Component: pgadmin Version: trunk
Keywords: browser trigger Cc:
Platform: all

Description

Brackets around the WHEN condition are missing or unmatched in the SQL pane. (See ticket #98)

pg feature applies to postgresql 9.0+
Tested with pgAdmin 1.14.0 Beta 3 on Win XP; pg 9.0.4 on Debian Squeeze.

Docs:
 http://www.postgresql.org/docs/9.0/interactive/sql-createtrigger.html

-- Test case --

CREATE TABLE foo(a serial, b text, c text, d text);

CREATE OR REPLACE FUNCTION trg_foo_upaft() RETURNS trigger AS
$BODY$
BEGIN

-- do something
RETURN NEW;

END;
$BODY$ LANGUAGE plpgsql VOLATILE;

-- Test 1.)
-- I say:
-- DROP TRIGGER up_aft ON foo;
CREATE TRIGGER up_aft

AFTER UPDATE
ON foo
FOR EACH ROW
WHEN ((old.b, old.c, old.d) <> (new.b, new.c, new.d))
EXECUTE PROCEDURE trg_foo_upaft();

--> pgAdmin says:
CREATE TRIGGER up_aft

AFTER UPDATE
ON foo
FOR EACH ROW
WHEN (old.b <> new.b) OR (old.c <> new.c) OR (old.d <> new.d)
EXECUTE PROCEDURE trg_foo_upaft();

--! no enclosing brackets !

-- Test 2.)
-- I say:
-- DROP TRIGGER up_aft ON foo;
CREATE TRIGGER up_aft

AFTER UPDATE
ON foo
FOR EACH ROW
WHEN ((old.b <> new.b) OR (old.c <> new.c) OR (old.d <> new.d))
EXECUTE PROCEDURE trg_foo_upaft();

--> pgAdmin says:
CREATE TRIGGER up_aft

AFTER UPDATE
ON foo
FOR EACH ROW
WHEN (old.b <> new.b) OR (old.c <> new.c)) OR (old.d <> new.d)
EXECUTE PROCEDURE trg_foo_upaft();

--! unmatched bracket and missing enclosing brackets !

Change History

Changed 22 months ago by brsa

  • type changed from bug to patch

The fix is pretty obvious in this case. The code mistakenly trims all outer enclosing brackets, which leads to unmatched brackets in test 2.) - both left brackets were removed.

pgTrigger.cpp, line 393:

- wxT(" trim(substring(pg_get_triggerdef(t.oid), 'WHEN (.*) EXECUTE PROCEDURE'), '()') AS whenclause\n")
+ wxT(" substring(pg_get_triggerdef(t.oid), 'WHEN (.*) EXECUTE PROCEDURE') AS whenclause\n")

Changed 21 months ago by gleu

  • status changed from new to accepted
  • owner changed from dpage to gleu
  • type changed from patch to bug

Changed 21 months ago by gleu

  • keywords browser trigger added; SQL pane removed
  • status changed from accepted to closed
  • resolution set to fixed
  • milestone set to 1.14

Changed 21 months ago by brsa

First off, I don't actually understand most of the code, I am only poking at a spot I found. My "code" is from the top of my head.

My patch apparently fixed the problem but left one set of enclosing brackets too many.
Your additional patch cuts first and last character from the WHEN-Expression.

In v1.14 RC1 I still see one set of brackets too many. So, somehow, this fails to work. Or maybe it does works but still leaves an extra set of brackets just like pg_get_triggerdef() does. (No idea why.)

In any case, I propose this simpler fix instead:
pgTrigger.cpp, line 393:

- wxT(" substring(pg_get_triggerdef(t.oid), 'WHEN (.*) EXECUTE PROCEDURE') AS whenclause\n")
+ wxT(" substring(pg_get_triggerdef(t.oid), E'WHEN \\((.*)\\) EXECUTE PROCEDURE') AS whenclause\n")

Changed 21 months ago by gleu

It doesn't fail, PostgreSQL send it to us this way. And it works. You can copy the statement to execute it, it works.

Your new fix won't work all the time. What we can do is use the new argument of pg_get_triggerdef that makes the definition prettier. That could be done.

Note: See TracTickets for help on using tickets.