Thread: [pgAdmin III] #332: SQL for TRIGGER with WHEN clause broken
#332: SQL for TRIGGER with WHEN clause broken ----------------------+----------------------------------------------------- Reporter: brsa | Owner: dpage Type: bug | Status: new Priority: minor | Milestone: Component: pgadmin | Version: trunk Keywords: SQL pane | Platform: all ----------------------+----------------------------------------------------- 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 ! -- Ticket URL: <http://code.pgadmin.org/trac/ticket/332> pgAdmin III <http://code.pgadmin.org/trac/> pgAdmin III
#332: SQL for TRIGGER with WHEN clause broken ----------------------+----------------------------------------------------- Reporter: brsa | Owner: dpage Type: patch | Status: new Priority: minor | Milestone: Component: pgadmin | Version: trunk Keywords: SQL pane | Platform: all ----------------------+----------------------------------------------------- Changes (by brsa): * type: bug => patch Comment: 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") -- Ticket URL: <http://code.pgadmin.org/trac/ticket/332#comment:1> pgAdmin III <http://code.pgadmin.org/trac/> pgAdmin III
#332: SQL for TRIGGER with WHEN clause broken ----------------------+----------------------------------------------------- Reporter: brsa | Owner: gleu Type: bug | Status: accepted Priority: minor | Milestone: Component: pgadmin | Version: trunk Keywords: SQL pane | Platform: all ----------------------+----------------------------------------------------- Changes (by gleu): * owner: dpage => gleu * status: new => accepted * type: patch => bug -- Ticket URL: <http://code.pgadmin.org/trac/ticket/332#comment:2> pgAdmin III <http://code.pgadmin.org/trac/> pgAdmin III
#332: SQL for TRIGGER with WHEN clause broken ----------------------+----------------------------------------------------- Reporter: brsa | Owner: gleu Type: bug | Status: closed Priority: minor | Milestone: 1.14 Component: pgadmin | Version: trunk Resolution: fixed | Keywords: browser trigger Platform: all | ----------------------+----------------------------------------------------- Changes (by gleu): * status: accepted => closed * keywords: SQL pane => browser trigger * resolution: => fixed * milestone: => 1.14 Comment: Your patch wasn't enough. We need to strip the first and last brackets, but the previous code did strip too much. See http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=ba8d4935060774d1340a42870b3140a99886dc85 and http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=4f4a24588973132989c129db3ddd14d48adeab23. -- Ticket URL: <http://code.pgadmin.org/trac/ticket/332#comment:3> pgAdmin III <http://code.pgadmin.org/trac/> pgAdmin III
#332: SQL for TRIGGER with WHEN clause broken ----------------------+----------------------------------------------------- Reporter: brsa | Owner: gleu Type: bug | Status: closed Priority: minor | Milestone: 1.14 Component: pgadmin | Version: trunk Resolution: fixed | Keywords: browser trigger Platform: all | ----------------------+----------------------------------------------------- Comment(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") -- Ticket URL: <http://code.pgadmin.org/trac/ticket/332#comment:4> pgAdmin III <http://code.pgadmin.org/trac/> pgAdmin III
#332: SQL for TRIGGER with WHEN clause broken ----------------------+----------------------------------------------------- Reporter: brsa | Owner: gleu Type: bug | Status: closed Priority: minor | Milestone: 1.14 Component: pgadmin | Version: trunk Resolution: fixed | Keywords: browser trigger Platform: all | ----------------------+----------------------------------------------------- Comment(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. -- Ticket URL: <http://code.pgadmin.org/trac/ticket/332#comment:5> pgAdmin III <http://code.pgadmin.org/trac/> pgAdmin III