Topology: LayerTrigger - wrong filter

[aperi2007]

Hi, The LayerTrigger function in topology section actually use this code sql:

    query = 'SELECT * '
             || ' FROM ' || quote_ident(toponame)
             || '.relation '
             || ' WHERE topogeo_id = ' || OLD.topology_id
             || ' AND layer_id = '|| OLD.layer_id
             || ' LIMIT 1';
         --RAISE NOTICE '%', query;

I guess the comparing between topogeo_id and topology_id is wrong. This could be allow/deny wrongly the deleting of some Layer. Also I guess the correct sql should be this:

  query = 'SELECT * '
             || ' FROM ' || quote_ident(toponame)
             || '.relation '
             || ' WHERE layer_id = '|| OLD.layer_id
             || ' LIMIT 1';

Because the topology is already the one when triggering this function.

regards, Andrea.

[strk]

confirmed. would need some work on a regress test.

[strk]

Andrea, I attached the work I've done on a regression test so far. Did not handle to trigger the bug yet, but maybe you can go on from there. I'm not going to work on this for at least 2 weeks and I know you planned to. So please go on from there and reattach a version when it makes the bug evident.

Thank you.

[aperi2007]

strk, there was a few of time but at last I found the combination to cause the FAIL of trigger.

select topology.DropTopology('t2');
select topology.DropTopology('t1');

select 'seq_reset', setval('topology.topology_id_seq', 1, false);
select 't1', topology.CreateTopology('t1');
insert into t1.node(geom, containing_face) values ('POINT( 0 0)', 0);
create table t1.l1 (id serial);
select 't1.l1',topology.AddTopoGeometryColumn('t1', 't1', 'l1', 'g', 'POINT');
insert INTO t1.l1(g) VALUES (topology.CreateTopoGeom('t1', 1, 1, '{{1,1}}'));

select 't2', topology.CreateTopology('t2');

insert into t2.node(geom, containing_face) values ('POINT( 0 0)', 0);
insert into t2.node(geom, containing_face) values ('POINT( 1 1)', 0);

create table t2.l1 (id serial);

select 't2.l1', topology.AddTopoGeometryColumn('t2', 't2', 'l1', 'g', 'POINT');

insert into t2.l1(g) VALUES (topology.CreateTopoGeom('t2', 1, 1, '{{1,1}}'));

-- the trigger FAILS because the t2.relation is not empty and should reject the delete
delete from t2.l1;

-- this next comand will remove the 't2' topology from topology.layer even if the t2.relation table is not empty.
-- the trigger should be reject the delete comand with "A record in t2.relation still references layer..."

delete from topology.layer where topology_id=2;

Now I can build the regress test.

[aperi2007]

patch for triggerlayer

[aperi2007]

strk,

I add to this ticket the patch with regress tests.

http://trac.osgeo.org/postgis/attachment/ticket/950/patch_layertrigger.zip

[strk]

Good job. I'm simplifying your testcase further. A question: does it sound good to you that those DELETE commands fail _silently_ ? I'm not sure I like it (unrelated to this specific bug, but got my attention)

[strk]

Oh, about the quiet issue, it's not really silent, but raises a NOTICE: NOTICE: A feature referencing layer 1 of topology 2 still exists in t2.l1.g

Should it be a WARNING instead ? Or, how about a plain ERROR ? I guess we should mimic the behavior of a proper foreign key constraint…

[strk]

Proper foreign keys raise an error: ERROR: update or delete on table "a" violates foreign key constraint "b_ref_fkey" on table "b"

[strk]

Your patch committed in r7514 and testcase further simplified in r7515.

It'll take another ticket to modify the trigger itself (but I don't plan to do that any time soon).