Proposal / proof of concept: Triggers on VIEWs

I've been thinking about the idea of allowing triggers on views as a
way of implementing updateable views. This would not be a replacement
for rules in all situations, but rather a complementary feature.
It would also complement the SQL standard feature allowing updates to
simple views, which is fine except that in practice I tend to have
more complex views, that would need custom code to update, and often
this is very difficult to achieve with rules.

The way this is done in some other databases is "INSTEAD OF"
triggers. These triggers are neither BEFORE nor AFTER, but
INSTEAD. And they are also typically ROW-level triggers, eg:

CREATE TRIGGER my_trig INSTEAD OF UPDATE ON my_view
FOR EACH ROW EXECUTE PROCEDURE my_trig_fn();

The trigger fires for each row that needs to be updated in the view,
and has access to OLD and NEW tuples containing all the columns from
the view. The trigger function takes full responsibility for updating
the base tables, which works OK provided that the view exposes
sufficient key columns to allow the relevant tuples in the base tables
to be identified.

I've used this feature in Oracle in the past, and found it very
useful. What I'm proposing is something similar to that. The main
restrictions placed on INSTEAD OF triggers in Oracle are:

1). They're only allowed on views not tables.
2). They may only be ROW-level triggers.
3). They don't support WHEN conditions.
4). They can't be used with for UPDATE OF <column list>.

1 and 2 could possibly be relaxed, but IMO they are sensible
restrictions at least in the first pass. 3 and 4 ensure that there is
an *unconditional* action to take instead of the VIEW update.

One thing that I think I would do differently from Oracle is the
following: in Oracle the return value of an INSTEAD OF trigger is
ignored, and it always just assumes that the trigger performed the
required update. In PostgreSQL, I think it might be more consistent
with the existing triggers to have the trigger return the OLD tuple
for DELETEs and the (possibly modified) NEW tuple for INSERT and
UPDATE. This would allow RETURNING clauses to show what was actually
added to the view by the trigger. A return value of NULL would
indicate that the trigger did nothing.

I've been thinking about how I would implement this, and attached is a
proof-of-concept patch. This patch doesn't actually have any trigger
definition or execution code (although I think writing that part
should be fairly mechanical). It's purpose is to get an idea of the
necessary changes to the rewriter, planner and executor. The patch
just raises NOTICEs in the executor at the point where the INSTEAD OF
triggers would be fired, showing what data would be available to those
triggers.

It's quite a small patch, which I hope isn't a sign that I've vastly
oversimplified this or overlooked something crucial. It works
basically as follows:

- In the rewriter, if the target of an UPDATE or DELETE is a VIEW,
then instead of simply replacing that view with its subquery, it now
adds the subquery to the end of the original query's rtable list, and
leaves the original RTE for the VIEW in place as the query's
resultRelation and for any returningList Vars. All other parts of the
query, including the jointree fromlist are modified to refer to the
VIEW's subquery, not the original VIEW RTE. For an INSERT, the
rewriter does nothing, leaving the VIEW RTE as the query
resultRelation.

- The planner largely ignores the VIEW relation RTE, since it does not
appear in the jointree fromlist. It is only used as the query target,
giving a plan with a ModifyTable node at the top and a subquery from
the view's base tables, joined to any other tables in the query,
together with any view conditions combined with any user conditions.

- In the executor, nodeModifyTable needs a few changes to be able to
handle a VIEW as the target relation. BEFORE and AFTER ROW triggers
are disallowed on views (I don't see a need for them if we have
INSTEAD OF triggers, and it's not clear how they would work anyway, in
the absence of a real table CTID). BEFORE and AFTER STATEMENT triggers
on the other hand, ought to work as-is. The INSTEAD OF triggers would
fire for each tuple coming from the subquery instead of the normal
heap_update/insert/delete().

Does this sound like a useful feature? Is this a sane approach to
implementing it? If not, has anyone else given any thought as to how
it might be implemented?

If this approach is valid, I believe that I should have time to put
together a more complete patch for the next commitfest.

Regards,
Dean

---

P.S. Here's some test output (note: the table is never actually updated
in this test, the NOTICEs just show what triggers would have done):

CREATE TABLE foo(a int PRIMARY KEY, b int, c int);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
"foo_pkey" for table "foo"
CREATE TABLE
INSERT INTO foo
SELECT g, g*10, g*100 FROM generate_series(1,10000) AS g;
INSERT 0 10000
CREATE VIEW foo_v AS
SELECT b AS bb, c AS cc, a AS aa FROM foo WHERE a%2 = 0;
CREATE VIEW
INSERT INTO foo_v (aa,bb,cc)
SELECT g, g*10, g*100 FROM generate_series(10001,10005) AS g;
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100010,
cc=1000100, aa=10001)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100020,
cc=1000200, aa=10002)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100030,
cc=1000300, aa=10003)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100040,
cc=1000400, aa=10004)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100050,
cc=1000500, aa=10005)
INSERT 0 5
UPDATE foo_v SET cc=0 WHERE aa < 10;
NOTICE: Trigger would UDPATE view "foo_v" OLD=(20,200,2) NEW=(bb=20,
cc=0, aa=2)
NOTICE: Trigger would UDPATE view "foo_v" OLD=(40,400,4) NEW=(bb=40,
cc=0, aa=4)
NOTICE: Trigger would UDPATE view "foo_v" OLD=(60,600,6) NEW=(bb=60,
cc=0, aa=6)
NOTICE: Trigger would UDPATE view "foo_v" OLD=(80,800,8) NEW=(bb=80,
cc=0, aa=8)
UPDATE 4
EXPLAIN UPDATE foo_v SET cc=0 WHERE aa < 10;
QUERY PLAN
---------------------------------------------------------------------------------
Update (cost=70.93..188.18 rows=18 width=18)
-> Bitmap Heap Scan on foo (cost=70.93..188.18 rows=18 width=18)
Recheck Cond: (a < 10)
Filter: ((a % 2) = 0)
-> Bitmap Index Scan on foo_pkey (cost=0.00..70.93 rows=3557 width=0)
Index Cond: (a < 10)
(6 rows)

DELETE FROM foo_v WHERE aa = 50;
NOTICE: Trigger would DELETE from view "foo_v" OLD=(500,5000,50)
DELETE 1
EXPLAIN DELETE FROM foo_v WHERE aa = 50;
QUERY PLAN
---------------------------------------------------------------------------
Delete (cost=0.00..8.27 rows=1 width=18)
-> Index Scan using foo_pkey on foo (cost=0.00..8.27 rows=1 width=18)
Index Cond: (a = 50)
Filter: ((a % 2) = 0)
(4 rows)

INSERT INTO foo_v (aa,bb,cc)
SELECT g, g*10, g*100 FROM generate_series(10001,10005) AS g
RETURNING aa,bb,cc;
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100010,
cc=1000100, aa=10001)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100020,
cc=1000200, aa=10002)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100030,
cc=1000300, aa=10003)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100040,
cc=1000400, aa=10004)
NOTICE: Trigger would INSERT into view "foo_v" NEW=(bb=100050,
cc=1000500, aa=10005)
aa | bb | cc
-------+--------+---------
10001 | 100010 | 1000100
10002 | 100020 | 1000200
10003 | 100030 | 1000300
10004 | 100040 | 1000400
10005 | 100050 | 1000500
(5 rows)

INSERT 0 5
CREATE VIEW foo_vv AS
SELECT cc AS c, aa AS a, bb AS b FROM foo_v WHERE aa%3 = 0;
CREATE VIEW
SELECT COUNT(*) FROM foo_vv;
count
-------
1666
(1 row)

SELECT * FROM foo_vv ORDER BY a LIMIT 5;
c | a | b
------+----+-----
600 | 6 | 60
1200 | 12 | 120
1800 | 18 | 180
2400 | 24 | 240
3000 | 30 | 300
(5 rows)

UPDATE foo_vv SET b=a*11 WHERE a=6;
NOTICE: Trigger would UDPATE view "foo_vv" OLD=(600,6,60) NEW=(c=600,
a=6, b=66)
UPDATE 1
EXPLAIN UPDATE foo_vv SET b=a*11 WHERE a=6;
QUERY PLAN
---------------------------------------------------------------------------
Update (cost=0.00..8.28 rows=1 width=18)
-> Index Scan using foo_pkey on foo (cost=0.00..8.28 rows=1 width=18)
Index Cond: (a = 6)
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(4 rows)

CREATE VIEW foo_vvv AS
SELECT a, b, (SELECT 1 FROM foo_vv AS vv2 WHERE vv2.a = vv1.b/12) AS c
FROM foo_vv AS vv1;
CREATE VIEW
DELETE FROM foo_vvv WHERE a=12;
NOTICE: Trigger would DELETE from view "foo_vvv" OLD=(12,120,)
DELETE 1
EXPLAIN DELETE FROM foo_vvv WHERE a=12;
QUERY PLAN
----------------------------------------------------------------------------------
Delete (cost=0.00..16.56 rows=1 width=14)
-> Index Scan using foo_pkey on foo (cost=0.00..16.56 rows=1 width=14)
Index Cond: (a = 12)
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
SubPlan 1
-> Index Scan using foo_pkey on foo (cost=0.00..8.28
rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(8 rows)

UPDATE foo_vvv SET c=NULL WHERE a=36;
NOTICE: Trigger would UDPATE view "foo_vvv" OLD=(36,360,1) NEW=(a=36,
b=360, c=null)
UPDATE 1
EXPLAIN UPDATE foo_vvv SET c=NULL WHERE a=36;
QUERY PLAN
----------------------------------------------------------------------------------
Update (cost=0.00..16.56 rows=1 width=14)
-> Index Scan using foo_pkey on foo (cost=0.00..16.56 rows=1 width=14)
Index Cond: (a = 36)
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
SubPlan 1
-> Index Scan using foo_pkey on foo (cost=0.00..8.28
rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(8 rows)

INSERT INTO foo_vvv VALUES (1,2,3);
NOTICE: Trigger would INSERT into view "foo_vvv" NEW=(a=1, b=2, c=3)
INSERT 0 1
EXPLAIN INSERT INTO foo_vvv VALUES (1,2,3);
QUERY PLAN
------------------------------------------------
Insert (cost=0.00..0.01 rows=1 width=0)
-> Result (cost=0.00..0.01 rows=1 width=0)
(2 rows)

CREATE RULE foo_vv_rule AS ON UPDATE TO foo_vv
DO INSTEAD SELECT 'foo_vv_rule doing nothing';
CREATE RULE
UPDATE foo_vv SET b=a*11 WHERE a=6;
?column?
---------------------------
foo_vv_rule doing nothing
(1 row)

UPDATE 0
EXPLAIN UPDATE foo_vv SET b=a*11 WHERE a=6;
QUERY PLAN
--------------------------------------------------------------------
Index Scan using foo_pkey on foo (cost=0.00..8.28 rows=1 width=0)
Index Cond: (a = 6)
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(3 rows)

UPDATE foo_vvv SET b=a*11 WHERE a=6;
NOTICE: Trigger would UDPATE view "foo_vvv" OLD=(6,60,) NEW=(a=6, b=66, c=null)
UPDATE 1
EXPLAIN UPDATE foo_vvv SET b=a*11 WHERE a=6;
QUERY PLAN
----------------------------------------------------------------------------------
Update (cost=0.00..24.84 rows=1 width=14)
-> Index Scan using foo_pkey on foo (cost=0.00..24.84 rows=1 width=14)
Index Cond: (a = 6)
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
SubPlan 1
-> Index Scan using foo_pkey on foo (cost=0.00..8.28
rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
SubPlan 2
-> Index Scan using foo_pkey on foo (cost=0.00..8.28
rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(12 rows)

CREATE RULE foo_vvv_rule AS ON UPDATE TO foo_vvv
DO INSTEAD UPDATE foo_v SET bb=NEW.b, cc=NEW.c WHERE aa=OLD.a;
CREATE RULE
UPDATE foo_vvv SET b=a*11 WHERE a >= 30 AND a <= 36;
NOTICE: Trigger would UDPATE view "foo_v" OLD=(300,3000,30)
NEW=(bb=330, cc=null, aa=30)
NOTICE: Trigger would UDPATE view "foo_v" OLD=(360,3600,36)
NEW=(bb=396, cc=1, aa=36)
UPDATE 2
EXPLAIN UPDATE foo_vvv SET b=a*11 WHERE a >= 30 AND a <= 36;
QUERY PLAN
------------------------------------------------------------------------------------
Update (cost=4.78..79.30 rows=1 width=32)
-> Nested Loop (cost=4.78..79.30 rows=1 width=32)
-> Bitmap Heap Scan on foo (cost=4.78..62.73 rows=1 width=14)
Recheck Cond: ((a >= 30) AND (a <= 36))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
-> Bitmap Index Scan on foo_pkey (cost=0.00..4.78
rows=53 width=0)
Index Cond: ((a >= 30) AND (a <= 36))
-> Index Scan using foo_pkey on foo (cost=0.00..8.27 rows=1 width=18)
Index Cond: (a = public.foo.a)
Filter: ((a % 2) = 0)
SubPlan 1
-> Index Scan using foo_pkey on foo (cost=0.00..8.28
rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(14 rows)

CREATE OR REPLACE RULE foo_vvv_rule AS ON UPDATE TO foo_vvv
DO INSTEAD UPDATE foo_v SET bb=NEW.b, cc=NEW.c WHERE aa=OLD.a
RETURNING NEW.a, NEW.b, NEW.c;
CREATE RULE
UPDATE foo_vvv SET b=a*11 WHERE a >= 30 AND a <= 36 RETURNING a, b, c;
NOTICE: Trigger would UDPATE view "foo_v" OLD=(300,3000,30)
NEW=(bb=330, cc=null, aa=30)
NOTICE: Trigger would UDPATE view "foo_v" OLD=(360,3600,36)
NEW=(bb=396, cc=1, aa=36)
a | b | c
----+-----+---
30 | 330 |
36 | 396 | 1
(2 rows)

UPDATE 2
EXPLAIN UPDATE foo_vvv SET b=a*11 WHERE a >= 30 AND a <= 36 RETURNING a, b, c;
QUERY PLAN
------------------------------------------------------------------------------------
Update (cost=4.78..79.30 rows=1 width=32)
-> Nested Loop (cost=4.78..79.30 rows=1 width=32)
-> Bitmap Heap Scan on foo (cost=4.78..62.73 rows=1 width=14)
Recheck Cond: ((a >= 30) AND (a <= 36))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
-> Bitmap Index Scan on foo_pkey (cost=0.00..4.78
rows=53 width=0)
Index Cond: ((a >= 30) AND (a <= 36))
-> Index Scan using foo_pkey on foo (cost=0.00..8.27 rows=1 width=18)
Index Cond: (a = public.foo.a)
Filter: ((a % 2) = 0)
SubPlan 1
-> Index Scan using foo_pkey on foo (cost=0.00..8.28
rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
SubPlan 2
-> Index Scan using foo_pkey on foo (cost=0.00..8.28 rows=1 width=0)
Index Cond: (a = (public.foo.b / 12))
Filter: (((a % 2) = 0) AND ((a % 3) = 0))
(18 rows)

On 8/4/10 2:39 PM +0300, Dean Rasheed wrote:
> Does this sound like a useful feature? Is this a sane approach to
> implementing it? If not, has anyone else given any thought as to how
> it might be implemented?

I didn't look at the patch, but so far, I've identified three problems
with the existing view system:

1) You can't re-evaluate the UPDATE expression like an UPDATE on a
table does. Consider for example UPDATE foo SET a=a+1; If the
tuples change before we get to them, we lose data because we
simply can't re-evaluate "a+1" in the trigger.

2) You can't set the number of affected rows.

3) You can't set the RETURNING results. You suggested that
RETURNING for DELETE would return the OLD value, but that seems
broken because that's not necessarily what was deleted. I didn't
understand what you suggestion for UPDATE was; how does PG know
that if the view doesn't have a primary key?

I think these are the main three problems that prevent people from
actually using views, and I think these should be focused on when adding
triggers on VIEWS. I would love to see the feature though.

Any thoughts?

Regards,
Marko Tiikkaja

On 4 August 2010 13:22, Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
> On 8/4/10 2:39 PM +0300, Dean Rasheed wrote:
>>
>> Does this sound like a useful feature? Is this a sane approach to
>> implementing it? If not, has anyone else given any thought as to how
>> it might be implemented?
>
> I didn't look at the patch, but so far, I've identified three problems with
> the existing view system:
>
>    1) You can't re-evaluate the UPDATE expression like an UPDATE on a
>       table does.  Consider for example  UPDATE foo SET a=a+1;  If the
>       tuples change before we get to them, we lose data because we
>       simply can't re-evaluate "a+1" in the trigger.
>

Is this the same problem the writeable CTE patch ran into?
The way I've done this, the OLD values passed to the trigger all come
from a snapshot established at the start of the query, so you're
right, the trigger won't see values changed after the query started,
unless it re-queries for them. I don't see an easy way round that.

>    2) You can't set the number of affected rows.
>

Yeah, the assumption is that the number of affected rows is the number
of rows in the view that matched the user's WHERE clause. You could
return fewer affected rows by having the trigger return NULL for some
of them, but you can't say that you've affected more than that. So
even if the trigger updates 10 rows in the base tables for a given row
in the view, that still only counts as 1 row affected in the view by
the original query.

>    3) You can't set the RETURNING results.  You suggested that
>       RETURNING for DELETE would return the OLD value, but that seems
>       broken because that's not necessarily what was deleted.

Well that's what happens for a table. Alternatively the trigger could
modify OLD, and then have RETURNING return that, but that's not what
happens in a BEFORE DELETE trigger on a table.

>  I didn't
>       understand what you suggestion for UPDATE was; how does PG know
>       that if the view doesn't have a primary key?

For INSERT and UPDATE the trigger would compute and make the necessary
changes to the base tables, and then return the new contents of the
view's row in a modified copy of NEW, if necessary for RETURNING. This
might include re-computed derived values for example.

If the view doesn't have a PK, or any other way of uniquely
identifying rows then its probably hopeless. That's not a case that
this patch is targeted for.

Regards,
Dean

>
> I think these are the main three problems that prevent people from actually
> using views, and I think these should be focused on when adding triggers on
> VIEWS.  I would love to see the feature though.
>
> Any thoughts?
>
>
> Regards,
> Marko Tiikkaja
>

On 8/4/10 4:31 PM +0300, Dean Rasheed wrote:
>> 1) You can't re-evaluate the UPDATE expression like an UPDATE on a
>> table does. Consider for example UPDATE foo SET a=a+1; If the
>> tuples change before we get to them, we lose data because we
>> simply can't re-evaluate "a+1" in the trigger.
>>
>
> Is this the same problem the writeable CTE patch ran into?

No, that was something different.

> Yeah, the assumption is that the number of affected rows is the number
> of rows in the view that matched the user's WHERE clause. You could
> return fewer affected rows by having the trigger return NULL for some
> of them, but you can't say that you've affected more than that. So
> even if the trigger updates 10 rows in the base tables for a given row
> in the view, that still only counts as 1 row affected in the view by
> the original query.

I think that's fine.

>> 3) You can't set the RETURNING results. You suggested that
>> RETURNING for DELETE would return the OLD value, but that seems
>> broken because that's not necessarily what was deleted.
>
> Well that's what happens for a table. Alternatively the trigger could
> modify OLD, and then have RETURNING return that, but that's not what
> happens in a BEFORE DELETE trigger on a table.

I'm not sure I understand. RETURNING in DELETE on a table fetches the
old value after it was DELETEd, so it really is what the tuple was
before the DLETE, not what is seen by the snapshot. In a BEFORE DELETE
trigger, the row is always locked so it can't change after the trigger
is fired.

> For INSERT and UPDATE the trigger would compute and make the necessary
> changes to the base tables, and then return the new contents of the
> view's row in a modified copy of NEW, if necessary for RETURNING. This
> might include re-computed derived values for example.

I see.

Regards,
Marko Tiikkaja

On 4 August 2010 14:43, Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
>>>    3) You can't set the RETURNING results.  You suggested that
>>>       RETURNING for DELETE would return the OLD value, but that seems
>>>       broken because that's not necessarily what was deleted.
>>
>> Well that's what happens for a table. Alternatively the trigger could
>> modify OLD, and then have RETURNING return that, but that's not what
>> happens in a BEFORE DELETE trigger on a table.
>
> I'm not sure I understand.  RETURNING in DELETE on a table fetches the old
> value after it was DELETEd, so it really is what the tuple was before the
> DLETE, not what is seen by the snapshot.  In a BEFORE DELETE trigger, the
> row is always locked so it can't change after the trigger is fired.
>

Ah, I think I mis-understood. If I understand what you're saying
correctly, you're worried that the row might have been modified in the
same query, prior to being deleted, and you want RETURNING to return
the updated value, as it was when it was deleted.

So yes, you're right, that really is different from a table. I guess
it would have to be handled by the trigger returning a modified copy
of OLD for RETURNING to use.

Regards,
Dean

On 8/4/10 5:03 PM +0300, Dean Rasheed wrote:
> On 4 August 2010 14:43, Marko Tiikkaja<marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
>> I'm not sure I understand. RETURNING in DELETE on a table fetches the old
>> value after it was DELETEd, so it really is what the tuple was before the
>> DLETE, not what is seen by the snapshot. In a BEFORE DELETE trigger, the
>> row is always locked so it can't change after the trigger is fired.
>>
>
> Ah, I think I mis-understood. If I understand what you're saying
> correctly, you're worried that the row might have been modified in the
> same query, prior to being deleted, and you want RETURNING to return
> the updated value, as it was when it was deleted.

I'm mainly concerned about concurrently running transactions.

Regards,
Marko Tiikkaja

On 4 August 2010 15:08, Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
> On 8/4/10 5:03 PM +0300, Dean Rasheed wrote:
>>
>> On 4 August 2010 14:43, Marko Tiikkaja<marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi>
>>  wrote:
>>>
>>> I'm not sure I understand.  RETURNING in DELETE on a table fetches the
>>> old
>>> value after it was DELETEd, so it really is what the tuple was before the
>>> DLETE, not what is seen by the snapshot.  In a BEFORE DELETE trigger, the
>>> row is always locked so it can't change after the trigger is fired.
>>>
>>
>> Ah, I think I mis-understood. If I understand what you're saying
>> correctly, you're worried that the row might have been modified in the
>> same query, prior to being deleted, and you want RETURNING to return
>> the updated value, as it was when it was deleted.
>
> I'm mainly concerned about concurrently running transactions.
>

Sorry for the delay replying.

Once again, I think I mis-understood your point. I think that the
database can't really lock anything before firing the trigger because
the view might contain grouping/aggregates or even not be based on any
real tables at all, so it would be impossible to work out what to
lock. Thus it would be up to the trigger function to get this right.
In the simplest case, for a DELETE, this might look something like:

CREATE OR REPLACE FUNCTION instead_of_delete_trig_fn()
RETURNS trigger AS
$$
BEGIN
DELETE FROM base_table WHERE pk = OLD.pk;
IF NOT FOUND THEN RETURN NULL; END IF;

RETURN OLD;
END;
$$
LANGUAGE plpgsql;

If 2 users try to delete the same row, the second would block until
the first user's transaction finished, and if the first user
committed, the second user's trigger would return NULL, which the
database would signal as no rows deleted.

Regards,
Dean

aoa how can i connect my postgres database to manifold?

Regards?

On Fri, Aug 6, 2010 at 12:49 PM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>wrote:

> On 4 August 2010 15:08, Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi>
> wrote:
> > On 8/4/10 5:03 PM +0300, Dean Rasheed wrote:
> >>
> >> On 4 August 2010 14:43, Marko Tiikkaja<marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi>
> >> wrote:
> >>>
> >>> I'm not sure I understand. RETURNING in DELETE on a table fetches the
> >>> old
> >>> value after it was DELETEd, so it really is what the tuple was before
> the
> >>> DLETE, not what is seen by the snapshot. In a BEFORE DELETE trigger,
> the
> >>> row is always locked so it can't change after the trigger is fired.
> >>>
> >>
> >> Ah, I think I mis-understood. If I understand what you're saying
> >> correctly, you're worried that the row might have been modified in the
> >> same query, prior to being deleted, and you want RETURNING to return
> >> the updated value, as it was when it was deleted.
> >
> > I'm mainly concerned about concurrently running transactions.
> >
>
> Sorry for the delay replying.
>
> Once again, I think I mis-understood your point. I think that the
> database can't really lock anything before firing the trigger because
> the view might contain grouping/aggregates or even not be based on any
> real tables at all, so it would be impossible to work out what to
> lock. Thus it would be up to the trigger function to get this right.
> In the simplest case, for a DELETE, this might look something like:
>
> CREATE OR REPLACE FUNCTION instead_of_delete_trig_fn()
> RETURNS trigger AS
> $$
> BEGIN
> DELETE FROM base_table WHERE pk = OLD.pk;
> IF NOT FOUND THEN RETURN NULL; END IF;
>
> RETURN OLD;
> END;
> $$
> LANGUAGE plpgsql;
>
> If 2 users try to delete the same row, the second would block until
> the first user's transaction finished, and if the first user
> committed, the second user's trigger would return NULL, which the
> database would signal as no rows deleted.
>
> Regards,
> Dean
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On 8/6/2010 10:49 AM, Dean Rasheed wrote:
> On 4 August 2010 15:08, Marko Tiikkaja<marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
>> I'm mainly concerned about concurrently running transactions.
>
> Once again, I think I mis-understood your point. I think that the
> database can't really lock anything before firing the trigger because
> the view might contain grouping/aggregates or even not be based on any
> real tables at all, so it would be impossible to work out what to
> lock.

Right.

> Thus it would be up to the trigger function to get this right.
> In the simplest case, for a DELETE, this might look something like:
>
> CREATE OR REPLACE FUNCTION instead_of_delete_trig_fn()
> RETURNS trigger AS
> $$
> BEGIN
> DELETE FROM base_table WHERE pk = OLD.pk;
> IF NOT FOUND THEN RETURN NULL; END IF;
>
> RETURN OLD;
> END;
> $$
> LANGUAGE plpgsql;
>
> If 2 users try to delete the same row, the second would block until
> the first user's transaction finished, and if the first user
> committed, the second user's trigger would return NULL, which the
> database would signal as no rows deleted.

The problem is that this isn't even nearly sufficient. I gave this some
more thought while I was away, and it seems that I missed at least one
more important thing: the WHERE clause. Imagine this query:

DELETE FROM view WHERE pk = 1 AND f1 > 0;

Now the trigger function gets called if the row where pk = 1, as seen by
the query's snapshot, has f1 > 0. But if a concurrent transaction sets
f1 to 0 before the triggers gets to the row, you end up deleting a row
that doesn't match the WHERE clause. I have a few ideas on how this
could be tackled, but I think we need to split these two threads. I
still think that having triggers on views without addressing these
concurrency concerns is not a good idea, though.

Regards,
Marko Tiikkaja

On 7 August 2010 10:56, Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
> The problem is that this isn't even nearly sufficient.  I gave this some
> more thought while I was away, and it seems that I missed at least one more
> important thing: the WHERE clause.  Imagine this query:
>
> DELETE FROM view WHERE pk = 1 AND f1 > 0;
>
> Now the trigger function gets called if the row where pk = 1, as seen by the
> query's snapshot, has f1 > 0.  But if a concurrent transaction sets f1 to 0
> before the triggers gets to the row, you end up deleting a row that doesn't
> match the WHERE clause.

I've been playing with this in Oracle and I can confirm that it behaves
exactly as my code would do. So in this example, the trigger deletes
from the underlying table even after the row has been changed so that
it no longer satisfies the original WHERE clause. The case I find
worse is that if 2 concurrent transactions do UPDATE view SET f1=f1+1,
the value will only be incremented once.

For the record, here is the Oracle code in all its gory detail:

CREATE TABLE foo(a int, b int);
CREATE VIEW foo_v AS SELECT * FROM foo;

CREATE TRIGGER del_trig INSTEAD OF DELETE ON foo_v
FOR EACH ROW
BEGIN
DELETE FROM foo WHERE a=:OLD.a;
END;
/

CREATE TRIGGER mod_trig INSTEAD OF UPDATE ON foo_v
FOR EACH ROW
BEGIN
UPDATE foo SET a=:NEW.a, b=:NEW.b WHERE a=:OLD.a;
END;
/

INSERT INTO foo VALUES(1,1);
COMMIT;

So in the first case, 2 concurrent transactions T1 and T2 are started
and do the following:

1. T1 does UPDATE foo_v SET b=0 WHERE a=1;
-- Trigger fires and updates foo
2. T2 does DELETE FROM foo_v WHERE a=1 AND b=1;
-- This matches 1 row in the VIEW (T1 not committed yet)
-- Trigger fires and does DELETE FROM foo WHERE a=1
-- T2 waits for T1
3. T1 commits
4. T2 resumes and deletes the row from foo, since it satisfies a=1

Arguably, this is a deficiency in the trigger function rather than the
trigger firing code. It could be fixed by having the trigger re-check
all the columns in the table against OLD, but that would be pretty
cumbersome for very wide tables, and none of the documented
examples I've seen take that approach.

The second case is as follows:

INSERT INTO foo VALUES(1,1);
COMMIT;

Then 2 concurrent transactions T1 and T2 are started and do the
following:

1. T1 does UPDATE foo_v SET b=b+1 WHERE a=1;
-- Trigger fires and does UPDATE foo SET b=2 WHERE a=1;
2. T2 does UDPATE foo_v SET b=b+1 WHERE a=1;
-- Trigger fires and does UPDATE foo SET b=2 WHERE a=1;
-- T2 waits for T1
3. T1 commits
4. T2 resumes and sets b to 2 as requested inside the trigger

So the net result is b=2 rather than b=3 - pretty-much the textbook
concurrency example. Surprisingly I don't see too many people
complaining about this in Oracle, because to me it seems pretty bad.

In PostgreSQL we could fix it by declaring the VIEW query as FOR
UPDATE, but that's no good if for example the VIEW was based on an
aggregate. Oracle has the same limitations on FOR UPDATE, but also
AFAICS it doesn't allow VIEWs to be created using SELECT FOR UPDATE
at all.

For those migrating code from Oracle, providing this feature as-is
might be valuable, since presumably they are not too concerned about
these concurrency issues. Ideally we'd want to do better though.

Thoughts?

Regards,
Dean

> I have a few ideas on how this could be tackled,
> but I think we need to split these two threads.  I still think that having
> triggers on views without addressing these concurrency concerns is not a
> good idea, though.
>
>
> Regards,
> Marko Tiikkaja
>

On 8/8/2010 12:49 PM, Dean Rasheed wrote:
> On 7 August 2010 10:56, Marko Tiikkaja<marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
>> The problem is that this isn't even nearly sufficient. I gave this some
>> more thought while I was away, and it seems that I missed at least one more
>> important thing: the WHERE clause. Imagine this query:
>>
>> DELETE FROM view WHERE pk = 1 AND f1> 0;
>>
>> Now the trigger function gets called if the row where pk = 1, as seen by the
>> query's snapshot, has f1> 0. But if a concurrent transaction sets f1 to 0
>> before the triggers gets to the row, you end up deleting a row that doesn't
>> match the WHERE clause.
>
> I've been playing with this in Oracle and I can confirm that it behaves
> exactly as my code would do. So in this example, the trigger deletes
> from the underlying table even after the row has been changed so that
> it no longer satisfies the original WHERE clause. The case I find
> worse is that if 2 concurrent transactions do UPDATE view SET f1=f1+1,
> the value will only be incremented once.

Wow. I'm surprised to hear this.

> In PostgreSQL we could fix it by declaring the VIEW query as FOR
> UPDATE, but that's no good if for example the VIEW was based on an
> aggregate. Oracle has the same limitations on FOR UPDATE, but also
> AFAICS it doesn't allow VIEWs to be created using SELECT FOR UPDATE
> at all.

Also making all SELECTs on the view FOR UPDATE is not a very good idea..

> For those migrating code from Oracle, providing this feature as-is
> might be valuable, since presumably they are not too concerned about
> these concurrency issues. Ideally we'd want to do better though.

Yes, you might be right. This feature on its on can greatly simplify
what people now have to do to get triggers on views.

Regards,
Marko Tiikkaja

On 2010-08-08 1:45 PM +0300, I wrote:
> On 8/8/2010 12:49 PM, Dean Rasheed wrote:
>> For those migrating code from Oracle, providing this feature as-is
>> might be valuable, since presumably they are not too concerned about
>> these concurrency issues. Ideally we'd want to do better though.
>
> Yes, you might be right. This feature on its on can greatly simplify
> what people now have to do to get triggers on views.

This weekend I had the luxury of working with our VIEW system, and it
seems like I greatly underestimated the power of this feature, even if
the concurrency issues I've mentioned in this thread would be left
unaddressed.

I haven't looked at the patch yet (I plan to do so), but +100 for this
feature.

Should we expect a new patch any time soon?

Regards,
Marko Tiikkaja

On 14 August 2010 13:12, Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> wrote:
> On 2010-08-08 1:45 PM +0300, I wrote:
>>
>> On 8/8/2010 12:49 PM, Dean Rasheed wrote:
>>>
>>> For those migrating code from Oracle, providing this feature as-is
>>> might be valuable, since presumably they are not too concerned about
>>> these concurrency issues. Ideally we'd want to do better though.
>>
>> Yes, you might be right.  This feature on its on can greatly simplify
>> what people now have to do to get triggers on views.
>
> This weekend I had the luxury of working with our VIEW system, and it seems
> like I greatly underestimated the power of this feature, even if the
> concurrency issues I've mentioned in this thread would be left unaddressed.
>
> I haven't looked at the patch yet (I plan to do so), but +100 for this
> feature.
>

Thanks for your feedback. I'd be interested in any ideas on the
concurrency issues.

> Should we expect a new patch any time soon?
>

I hope to look at this again tomorrow.
I'll try to post an updated patch then, with some real trigger code,
but without addressing any of the concurrency issues.

Regards,
Dean

On 14 August 2010 23:22, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> I'll try to post an updated patch then, with some real trigger code,
>

I've moved this to a new thread, with a WIP patch that allow 3 types
of triggers to be added to VIEWs:
http://archives.postgresql.org/pgsql-hackers/2010-08/msg01030.php

Comments welcome.

Cheers,
Dean

On Mon, Aug 16, 2010 at 09:05:12AM +0100, Dean Rasheed wrote:
> On 14 August 2010 23:22, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> > I'll try to post an updated patch then, with some real trigger
> > code,
>
> I've moved this to a new thread, with a WIP patch that allow 3 types
> of triggers to be added to VIEWs:
> http://archives.postgresql.org/pgsql-hackers/2010-08/msg01030.php
>
> Comments welcome.

Please add this to the next commitfest :)

https://commitfest.postgresql.org/action/commitfest_view?id=7

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On 16 August 2010 14:45, David Fetter <david(at)fetter(dot)org> wrote:
> Please add this to the next commitfest :)
>

Done.

Thanks,
Dean

> https://commitfest.postgresql.org/action/commitfest_view?id=7
>
> Cheers,
> David.
> --
> David Fetter <david(at)fetter(dot)org> http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter      XMPP: david(dot)fetter(at)gmail(dot)com
> iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>