Concurrently updating an updatable view

Hi developers,

Concurrently updating an updatable view seems to cause
an unexpected result. Is it a known issue?

=> select version();
version
-------------------------------------------------------------------
PostgreSQL 8.2.1 on i686-pc-mingw32, compiled by GCC gcc.exe (GCC)
3.4.4 (mingw special)
(1 row)

=> create table test (id int4 primary key, dt text)
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
"test_pkey" for table "test"
CREATE TABLE
=> insert into test values (1, 'a');
INSERT 0 1

=> create view test_v as select * from test;
CREATE VIEW
=> create rule test_upd as on update to test_v
do instead update test set dt=NEW.dt where id=OLD.id;
CREATE RULE

The result of concurrently running the same query
update test_v set dt='b' where dt='a'
is as follows.

session-1 => begin;
BEGIN
session-1 => update test_v set dt='b' where dt='a';
UPDATE 1

session-2 => begin;
BEGIN
session-2 => update test_v set dt='b' where dt='a';
(blocked)

session-1 => commit;
COMMIT

(session-2)
UPDATE 1

*Explain* shows the following plan for the query here.
=> explain update test_v set dt='b' where dt='a';
QUERY PLAN
-----------------------------------------------------------------
Hash Join (cost=24.57..50.59 rows=6 width=10)
Hash Cond: (public.test.id = public.test.id)
-> Seq Scan on test (cost=0.00..21.60 rows=1160 width=10)
-> Hash (cost=24.50..24.50 rows=6 width=4)
-> Seq Scan on test (cost=0.00..24.50 rows=6 width=4)
Filter: (dt = 'a'::text)
(6 rows)

regards,
Hiroshi Inoue

Hiroshi Inoue wrote:
> Concurrently updating an updatable view seems to cause
> an unexpected result. Is it a known issue?

Looks right to me. What did you expect?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> Hiroshi Inoue wrote:
>> Concurrently updating an updatable view seems to cause
>> an unexpected result. Is it a known issue?
>
> Looks right to me. What did you expect?

Shouldn't the last response
(session-2)
UPDATE 1

be
(seesion-2)
UPDATE 0
?

regards,
Hiroshi Inoue

Hiroshi Inoue wrote:
> Heikki Linnakangas wrote:
>> Hiroshi Inoue wrote:
>>> Concurrently updating an updatable view seems to cause
>>> an unexpected result. Is it a known issue?
>> Looks right to me. What did you expect?
>
> Shouldn't the last response
> (session-2)
> UPDATE 1
>
> be
> (seesion-2)
> UPDATE 0
> ?

Ah, I re-read the example and I see what you mean now.

The problem is that the new tuple version is checked only against the
condition in the update rule, id=OLD.id, but not the condition in the
original update-claus, dt='a'.

Yeah, that's confusing :(.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> The problem is that the new tuple version is checked only against the
> condition in the update rule, id=OLD.id, but not the condition in the
> original update-claus, dt='a'.
>
> Yeah, that's confusing :(.

Bit more than just normal rule confusion I'd say. Try the following two
statements in parallel (assuming you've just run the previous):

UPDATE test SET dt='c';
UPDATE test SET dt='x' FROM test t2 WHERE test.id=t2.id AND t2.dt='b';

This isn't a problem with the view mechanism - it's a problem with
re-checking clauses involving subqueries or joins I'd guess.

I'm trying to decide if it's unexpected or just plain wrong, and I think
I'd have to argue wrong.

--
Richard Huxton
Archonet Ltd

Richard Huxton wrote:
> Heikki Linnakangas wrote:
>> The problem is that the new tuple version is checked only against the
>> condition in the update rule, id=OLD.id, but not the condition in the
>> original update-claus, dt='a'.
>>
>> Yeah, that's confusing :(.
>
> Bit more than just normal rule confusion I'd say. Try the following two
> statements in parallel (assuming you've just run the previous):
>
> UPDATE test SET dt='c';
> UPDATE test SET dt='x' FROM test t2 WHERE test.id=t2.id AND t2.dt='b';
>
> This isn't a problem with the view mechanism - it's a problem with
> re-checking clauses involving subqueries or joins I'd guess.
>
> I'm trying to decide if it's unexpected or just plain wrong, and I think
> I'd have to argue wrong.

Or perhaps I'd not argue that :-/

This is really about MVCC in read committed mode, and the "just right
for simpler cases":
http://www.postgresql.org/docs/8.2/interactive/transaction-iso.html#XACT-READ-COMMITTED

Clearly there needs to be a change to the sentence: "Because of the
above rule, it is possible for an updating command to see an
inconsistent snapshot: it can see the effects of concurrent updating
commands that affected the same rows it is trying to update"

Not true if there's a subquery/join involved.

--
Richard Huxton
Archonet Ltd

Richard Huxton wrote:
> Richard Huxton wrote:
>> Heikki Linnakangas wrote:
>>> The problem is that the new tuple version is checked only against the
>>> condition in the update rule, id=OLD.id, but not the condition in the
>>> original update-claus, dt='a'.
>>>
>>> Yeah, that's confusing :(.
>>
>> Bit more than just normal rule confusion I'd say. Try the following
>> two statements in parallel (assuming you've just run the previous):
>>
>> UPDATE test SET dt='c';
>> UPDATE test SET dt='x' FROM test t2 WHERE test.id=t2.id AND t2.dt='b';
>>
>> This isn't a problem with the view mechanism - it's a problem with
>> re-checking clauses involving subqueries or joins I'd guess.
>>
>> I'm trying to decide if it's unexpected or just plain wrong, and I
>> think I'd have to argue wrong.
>
> Or perhaps I'd not argue that :-/
Well, src/backend/executor/README agrees with you that it's wrong..

"Note a fundamental bogosity of this approach: if the relation containing
the original tuple is being used in a self-join, the other instance(s) of
the relation will be treated as still containing the original tuple, whereas
logical consistency would demand that the modified tuple appear in them too.
But we'd have to actually substitute the modified tuple for the original,
while still returning all the rest of the relation, to ensure consistent
answers. Implementing this correctly is a task for future work."

> This is really about MVCC in read committed mode, and the "just right
> for simpler cases":
> http://www.postgresql.org/docs/8.2/interactive/transaction-iso.html#XACT-READ-COMMITTED
>
> Clearly there needs to be a change to the sentence: "Because of the
> above rule, it is possible for an updating command to see an
> inconsistent snapshot: it can see the effects of concurrent updating
> commands that affected the same rows it is trying to update"
>
> Not true if there's a subquery/join involved.
If the cited part of the README is correct, then all joins and subqueries
are fine, except if they refer to the table being updated.

I think there should be a big, fat warning that self-referential
updates have highly non-obvious behaviour in read-committed mode,
and should be avoided.

greetings, Florian Pflug

Richard Huxton wrote:
> Heikki Linnakangas wrote:
>> The problem is that the new tuple version is checked only against the
>> condition in the update rule, id=OLD.id, but not the condition in the
>> original update-claus, dt='a'.
>>
>> Yeah, that's confusing :(.
>
> Bit more than just normal rule confusion I'd say. Try the following two
> statements in parallel (assuming you've just run the previous):
>
> UPDATE test SET dt='c';
> UPDATE test SET dt='x' FROM test t2 WHERE test.id=t2.id AND t2.dt='b';
>
> This isn't a problem with the view mechanism - it's a problem with
> re-checking clauses involving subqueries or joins I'd guess.

I don't understand the PostgreSQL specific *FROM* clause correctly.
Currently the relations in the *FROM* clause seem to be read only
and UPDATE operations seem to acquire no tuple level lock on them.

regards,
Hiroshi Inoue

Hiroshi Inoue wrote:
> Richard Huxton wrote:
>> Heikki Linnakangas wrote:
>>> The problem is that the new tuple version is checked only against the
>>> condition in the update rule, id=OLD.id, but not the condition in the
>>> original update-claus, dt='a'.
>>>
>>> Yeah, that's confusing :(.
>>
>> Bit more than just normal rule confusion I'd say. Try the following
>> two statements in parallel (assuming you've just run the previous):
>>
>> UPDATE test SET dt='c';
>> UPDATE test SET dt='x' FROM test t2 WHERE test.id=t2.id AND t2.dt='b';
>>
>> This isn't a problem with the view mechanism - it's a problem with
>> re-checking clauses involving subqueries or joins I'd guess.
>
> I don't understand the PostgreSQL specific *FROM* clause correctly.
> Currently the relations in the *FROM* clause seem to be read only
> and UPDATE operations seem to acquire no tuple level lock on them.

Yes, the above query is equivalent to:
UPDATE test SET dt='x' WHERE id IN (SELECT id FROM test WHERE dt='b');

There are some expressions more naturally expressed as a set of where
conditions though, and I think the "FROM" is just to provide a place to
name them.

The FROM form seemed to be the more natural match to the plan your view
was generating - I'm not sure which the plan transformation process
produces.

--
Richard Huxton
Archonet Ltd

Florian G. Pflug wrote:
> Richard Huxton wrote:
>> Richard Huxton wrote:
>>> Heikki Linnakangas wrote:

<snip>

>>> Bit more than just normal rule confusion I'd say. Try the following
>>> two statements in parallel (assuming you've just run the previous):
>>>
>>> UPDATE test SET dt='c';
>>> UPDATE test SET dt='x' FROM test t2 WHERE test.id=t2.id AND t2.dt='b';
>>>
>>> This isn't a problem with the view mechanism - it's a problem with
>>> re-checking clauses involving subqueries or joins I'd guess.
>>>
>>> I'm trying to decide if it's unexpected or just plain wrong, and I
>>> think I'd have to argue wrong.
>>
>> Or perhaps I'd not argue that :-/

> Well, src/backend/executor/README agrees with you that it's wrong..

Thanks for the pointer.

> "Note a fundamental bogosity of this approach: if the relation containing
> the original tuple is being used in a self-join, the other instance(s) of
> the relation will be treated as still containing the original tuple,
> whereas
> logical consistency would demand that the modified tuple appear in them
> too.

Is the above description about UPDATE or DELETE operations?
AFAIR SELECT FOR UPDATE operations avoided the incosistency from the
first for joins though I'm not sure about subqueries.
Or I may be misunderstanding something?

> But we'd have to actually substitute the modified tuple for the original,
> while still returning all the rest of the relation, to ensure consistent
> answers. Implementing this correctly is a task for future work."

<snip>

> I think there should be a big, fat warning that self-referential
> updates have highly non-obvious behaviour in read-committed mode,
> and should be avoided.

It seems pretty difficult for PostgreSQL rule system to avoid such
kind of updates. I'm suspicious if UPDATABLE VIEWS can be implemented
using the rule system.

regards,
Hiroshi Inoue

Hiroshi Inoue wrote:
> Florian G. Pflug wrote:
>
>> I think there should be a big, fat warning that self-referential
>> updates have highly non-obvious behaviour in read-committed mode,
>> and should be avoided.
>
> It seems pretty difficult for PostgreSQL rule system to avoid such
> kind of updates. I'm suspicious if UPDATABLE VIEWS can be implemented
> using the rule system.

Remember this affects all self-referential joins on an UPDATE (and
DELETE?) not just views. It's just that a rule is more likely to produce
that type of query.

--
Richard Huxton
Archonet Ltd

Richard Huxton wrote:
> Hiroshi Inoue wrote:
>> Florian G. Pflug wrote:
>>
>>> I think there should be a big, fat warning that self-referential
>>> updates have highly non-obvious behaviour in read-committed mode,
>>> and should be avoided.
>>
>> It seems pretty difficult for PostgreSQL rule system to avoid such
>> kind of updates. I'm suspicious if UPDATABLE VIEWS can be implemented
>> using the rule system.
>
> Remember this affects all self-referential joins on an UPDATE (and
> DELETE?) not just views. It's just that a rule is more likely to produce
> that type of query.

Is there consensus what the correct behaviour should be for
self-referential updates in read-committed mode? Does the SQL Spec
have anything to say about this?

greetings, Florian Pflug

Florian G. Pflug wrote:
>
> Is there consensus what the correct behaviour should be for
> self-referential updates in read-committed mode? Does the SQL Spec
> have anything to say about this?

This seems to have gone all quiet. Do we need a TODO to keep a note of
it? Just "correct behaviour for self-referential updates"

Hiroshi originally noted the problem in one of his views here:
http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php

--
Richard Huxton
Archonet Ltd

Added to TODO:

* Fix self-referential UPDATEs seeing inconsistent row versions in
read-committed mode

http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php

---------------------------------------------------------------------------

Richard Huxton wrote:
> Florian G. Pflug wrote:
> >
> > Is there consensus what the correct behaviour should be for
> > self-referential updates in read-committed mode? Does the SQL Spec
> > have anything to say about this?
>
> This seems to have gone all quiet. Do we need a TODO to keep a note of
> it? Just "correct behaviour for self-referential updates"
>
> Hiroshi originally noted the problem in one of his views here:
> http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php
>
> --
> Richard Huxton
> Archonet Ltd
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Mon, 2007-05-28 at 19:56 -0400, Bruce Momjian wrote:
> Added to TODO:
>
> * Fix self-referential UPDATEs seeing inconsistent row versions in
> read-committed mode
>
> http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php
>

I'm sorry guys but I don't agree this is a TODO item.

IMHO this follows documented behaviour, even if y'all are shocked.

If you don't want the example cases to fail you can
- use SERIALIZABLE mode to throw an error if inconsistency is detected
- use SELECT FOR SHARE to lock the rows in the subselect
e.g.

UPDATE foo
SET pkcol = 'x'
WHERE pkcol IN
(SELECT pkcol
FROM foo
....
FOR SHARE);

In the case of concurrent UPDATEs the second UPDATE will normally
perform the subSELECT then hang waiting to perform the UPDATE. If you
use FOR SHARE the query will hang on the subSELECT (i.e. slightly
earlier), which makes the second query return zero rows, as some of you
were expecting.

Maybe we need a way of specifying that the non-UPDATE relation should be
locked FOR SHARE in a self-referencing UPDATE? Though that syntax could
seems to look pretty weird from here, so I'd say cover this situation in
a code example and be done.

Also, methinks we should have agreed behaviour before we make something
a TODO item. That would help us uncover this type of thing in more
detail, or at least force TODO to read "investigate whether ...".

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

Simon Riggs wrote:
> On Mon, 2007-05-28 at 19:56 -0400, Bruce Momjian wrote:
>> Added to TODO:
>>
>> * Fix self-referential UPDATEs seeing inconsistent row versions in
>> read-committed mode
>>
>> http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php
>>
>
> I'm sorry guys but I don't agree this is a TODO item.

Maybe the TODO suggested has a too narrow focus, but I think that
that *something* has to be done about this.

> IMHO this follows documented behaviour, even if y'all are shocked.
Yes, but documented != sensible && documented != intuitive &&
documented != logical.

> If you don't want the example cases to fail you can
> - use SERIALIZABLE mode to throw an error if inconsistency is detected
> - use SELECT FOR SHARE to lock the rows in the subselect
> e.g.
>
> UPDATE foo
> SET pkcol = 'x'
> WHERE pkcol IN
> (SELECT pkcol
> FROM foo
> ....
> FOR SHARE);
>
> In the case of concurrent UPDATEs the second UPDATE will normally
> perform the subSELECT then hang waiting to perform the UPDATE. If you
> use FOR SHARE the query will hang on the subSELECT (i.e. slightly
> earlier), which makes the second query return zero rows, as some of you
> were expecting.

Sure, but with a similar argument you could question the whole
update-in-read-committed-mode logic. After all, you wouldn't need
that logic if you always obtained a share lock on the rows to be updated
*before* you started updating them.

> Maybe we need a way of specifying that the non-UPDATE relation should be
> locked FOR SHARE in a self-referencing UPDATE? Though that syntax could
> seems to look pretty weird from here, so I'd say cover this situation in
> a code example and be done.
>
> Also, methinks we should have agreed behaviour before we make something
> a TODO item. That would help us uncover this type of thing in more
> detail, or at least force TODO to read "investigate whether ...".

Ack. Thats why I initially asked if there was consesus on what the
correct behaviour is.

greetings, Florian Pflug

"Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> I'm sorry guys but I don't agree this is a TODO item.
...
> Also, methinks we should have agreed behaviour before we make something
> a TODO item.

There is a whole *lot* of stuff in the TODO list that does not have a
consensus solution yet. You should not imagine that it's gospel.

At the same time, it'd be better if this item were worded more like
"investigate this issue" rather than presupposing a particular
form of answer.

regards, tom lane

Tom Lane wrote:
> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> > I'm sorry guys but I don't agree this is a TODO item.
> ...
> > Also, methinks we should have agreed behaviour before we make something
> > a TODO item.
>
> There is a whole *lot* of stuff in the TODO list that does not have a
> consensus solution yet. You should not imagine that it's gospel.
>
> At the same time, it'd be better if this item were worded more like
> "investigate this issue" rather than presupposing a particular
> form of answer.

OK, new wording:

o Research self-referential UPDATEs that see inconsistent row versions
in read-committed mode

http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Fri, 2007-06-01 at 20:06 +0200, Florian G. Pflug wrote:
> Simon Riggs wrote:
> > On Mon, 2007-05-28 at 19:56 -0400, Bruce Momjian wrote:
> >> Added to TODO:
> >>
> >> * Fix self-referential UPDATEs seeing inconsistent row versions in
> >> read-committed mode
> >>
> >> http://archives.postgresql.org/pgsql-hackers/2007-05/msg00507.php
> >>
> >
> > I'm sorry guys but I don't agree this is a TODO item.
>
> Maybe the TODO suggested has a too narrow focus, but I think that
> that *something* has to be done about this.
>
> > IMHO this follows documented behaviour, even if y'all are shocked.
> Yes, but documented != sensible && documented != intuitive &&
> documented != logical.

I've regrettably spent too many years with locking databases, so to me
the behaviour is perfectly logical, and intuitive, because I didn't
expect a lock avoiding strategy to be "free". We have a way of bypassing
locks, but there is no way to bypass the need to think about what the
answer will be if you do chose to bypass them.

This is just a classic database problem. Many books and courses bypass
these issues because they appear to be solved. Every couple of years
people go ballistic when they discover this kind of stuff exists; its
made the papers and SIGMOD too. I don't mean to dismiss everybody's
concerns, but it does seem I have a different perspective on them.

> > In the case of concurrent UPDATEs the second UPDATE will normally
> > perform the subSELECT then hang waiting to perform the UPDATE. If you
> > use FOR SHARE the query will hang on the subSELECT (i.e. slightly
> > earlier), which makes the second query return zero rows, as some of you
> > were expecting.
>
> Sure, but with a similar argument you could question the whole
> update-in-read-committed-mode logic. After all, you wouldn't need
> that logic if you always obtained a share lock on the rows to be updated
> *before* you started updating them.

Document it better if you will, or add an option that would allow you to
set the default to always use read locks, but there's just nothing
actually wrong with the current behaviour.

BTW, DB2 has just such an option, IIRC, but the equivalent "Repeatable
Read" (doesn't mean same thing as the ANSI phrase) mode is not
recommended for normal use. You'll then run up against the need to
optimise the locking further and end up with "Cursor Stability" mode.
Thats a lot of work for something few people will use in practice.

Read Committed == Show me the data, whether or not its been updated

Bear in mind this has nothing to do with self-referencing joins, its a
problem-of-misunderstanding of any UPDATE or DELETE with a sub-select
that references a table that can be updated concurrently.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

On Fri, 2007-06-01 at 14:12 -0400, Tom Lane wrote:
> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> > I'm sorry guys but I don't agree this is a TODO item.
> ...
> > Also, methinks we should have agreed behaviour before we make something
> > a TODO item.
>
> There is a whole *lot* of stuff in the TODO list that does not have a
> consensus solution yet. You should not imagine that it's gospel.

Well, I don't, though many think it is and some have been
surprised/annoyed to find out that implementing a TODO item doesn't mean
automatic acceptance of the idea, let alone the code (not myself, I
hasten to add).

> At the same time, it'd be better if this item were worded more like
> "investigate this issue" rather than presupposing a particular
> form of answer.

Agreed.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com