BUG #1231: Probelm with transactions in stored code.

The following bug has been logged online:

Bug reference: 1231
Logged by: Piotr Figiel

Email address: p(dot)figiel(at)aplok(dot)pl

PostgreSQL version: 7.4.3

Operating system: Linux Suse

Description: Probelm with transactions in stored code.

Details:

Hello
I have a problem with transactions in stored code in database.
This is testcase:

create table test_trans
( id numeric(4,0),
next_number numeric(4,0)
);

insert into test_trans values (1,1);

CREATE OR REPLACE FUNCTION test_tr() RETURNS numeric AS'
DECLARE
a numeric;
b numeric;
BEGIN
select next_number into b from test_trans where id=1;
update test_trans set next_number=next_number+1 where id=1;
select next_number into a from test_trans where id=1;

RETURN a ;
END;
'
LANGUAGE 'plpgsql' VOLATILE;

What I do then.
I've run two sessions.
In first I've run test_trans(), then in second I've run test_trans() too.
Second sessions waiting for first commit or rollback. Very good. Then I've
commited first session. What I see then:
First session returned value 2 - very good, but second session returned
value 1 - poor, oooo poor. Why , why, why? Second session should returned
value 3.
What happends. In version 8.0 Beta is the same situation. Additionl info:
I've must user read commited transacion isolation.
Please answer for my problem. My application based on this database but this
problem show everyone that PostgreSQL is not a transactional database.
Reagards
Piotr Figiel

On Wed, 25 Aug 2004, PostgreSQL Bugs List wrote:

> CREATE OR REPLACE FUNCTION test_tr() RETURNS numeric AS'
> DECLARE
> a numeric;
> b numeric;
> BEGIN
> select next_number into b from test_trans where id=1;
> update test_trans set next_number=next_number+1 where id=1;
> select next_number into a from test_trans where id=1;
>
> RETURN a ;
> END;
> '
> LANGUAGE 'plpgsql' VOLATILE;
>
> What I do then.
> I've run two sessions.
> In first I've run test_trans(), then in second I've run test_trans() too.
> Second sessions waiting for first commit or rollback. Very good. Then I've
> commited first session. What I see then:
> First session returned value 2 - very good, but second session returned
> value 1 - poor, oooo poor. Why , why, why? Second session should returned
> value 3.
> What happends. In version 8.0 Beta is the same situation. Additionl info:
> I've must user read commited transacion isolation.
> Please answer for my problem. My application based on this database but this
> problem show everyone that PostgreSQL is not a transactional database.

Actually, it shows that functions have odd behavior when locking is
involved (your statement would potentially be true if you could replicate
this without the functions). IIRC, there are issues currently with which
rows you see in such functions unless you end up using FOR UPDATE on the
selects or something of that sort.

Stephan Szabo wrote:

> On Wed, 25 Aug 2004, PostgreSQL Bugs List wrote:
>
>
>>CREATE OR REPLACE FUNCTION test_tr() RETURNS numeric AS'
>>DECLARE
>>a numeric;
>>b numeric;
>>BEGIN
>>select next_number into b from test_trans where id=1;
>>update test_trans set next_number=next_number+1 where id=1;
>>select next_number into a from test_trans where id=1;
>>
>>RETURN a ;
>>END;
>>'
>>LANGUAGE 'plpgsql' VOLATILE;
>>
>>What I do then.
>>I've run two sessions.
>>In first I've run test_trans(), then in second I've run test_trans() too.
>>Second sessions waiting for first commit or rollback. Very good. Then I've
>>commited first session. What I see then:
>>First session returned value 2 - very good, but second session returned
>>value 1 - poor, oooo poor. Why , why, why? Second session should returned
>>value 3.
>>What happends. In version 8.0 Beta is the same situation. Additionl info:
>>I've must user read commited transacion isolation.
>>Please answer for my problem. My application based on this database but this
>>problem show everyone that PostgreSQL is not a transactional database.
>
>
> Actually, it shows that functions have odd behavior when locking is
> involved (your statement would potentially be true if you could replicate
> this without the functions). IIRC, there are issues currently with which
> rows you see in such functions unless you end up using FOR UPDATE on the
> selects or something of that sort.

If the first select is a "FOR UPDATE" nothing change. For sure the last select in
that function doesn't see the same row if you perform that same select after
the function execution, and for sure doesn't see the same row that the update
statement touch.

Regards
Gaetano Mendola

On Thu, 26 Aug 2004, Gaetano Mendola wrote:

> Stephan Szabo wrote:
>
> > On Wed, 25 Aug 2004, PostgreSQL Bugs List wrote:
> >
> > Actually, it shows that functions have odd behavior when locking is
> > involved (your statement would potentially be true if you could replicate
> > this without the functions). IIRC, there are issues currently with which
> > rows you see in such functions unless you end up using FOR UPDATE on the
> > selects or something of that sort.
>
> If the first select is a "FOR UPDATE" nothing change. For sure the last select in

Right, I changed both to see if that made it "work" for me and it did. I
didn't bother to try the only after one.

> that function doesn't see the same row if you perform that same select after
> the function execution, and for sure doesn't see the same row that the update
> statement touch.

I believe it sees the one that was valid in the snapshot as of the
beginning of the function.

Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
> I believe it sees the one that was valid in the snapshot as of the
> beginning of the function.

Actually, the problem is that it can see *both* that row and the updated
row; it's a crapshoot which one will be returned by the SELECT INTO.

The reason this can happen is that we're not doing SetQuerySnapshot
between commands of a plpgsql function. There is discussion going way
way back about whether we shouldn't do so (see the archives). I think
the major reason why we have not done it is fear of introducing
non-backwards-compatible behavior. Seems like 8.0 is exactly the right
version to consider doing that in.

regards, tom lane

Tom Lane wrote:

> Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
>
>>I believe it sees the one that was valid in the snapshot as of the
>>beginning of the function.
>
>
> Actually, the problem is that it can see *both* that row and the updated
> row; it's a crapshoot which one will be returned by the SELECT INTO.

Confirmed, if the last select is:

select count(*) into a from test where id=1;

this return 2. There is a space for a new bug considering that if the
table have the unique index on id that select must return 1.

> The reason this can happen is that we're not doing SetQuerySnapshot
> between commands of a plpgsql function. There is discussion going way
> way back about whether we shouldn't do so (see the archives). I think
> the major reason why we have not done it is fear of introducing
> non-backwards-compatible behavior. Seems like 8.0 is exactly the right
> version to consider doing that in.

If my 2 cents are valid I agree with you, what I don't totally agree is why
consider this bug as a *feature* in previous 8.0 version.

Regards
Gaetano Mendola

On Thu, 2004-08-26 at 04:23, Gaetano Mendola wrote:
> Tom Lane wrote:
>
> > Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
> >
> >>I believe it sees the one that was valid in the snapshot as of the
> >>beginning of the function.
> >
> >
> > Actually, the problem is that it can see *both* that row and the updated
> > row; it's a crapshoot which one will be returned by the SELECT INTO.
>
> Confirmed, if the last select is:
>
> select count(*) into a from test where id=1;
>
> this return 2. There is a space for a new bug considering that if the
> table have the unique index on id that select must return 1.
>
> > The reason this can happen is that we're not doing SetQuerySnapshot
> > between commands of a plpgsql function. There is discussion going way
> > way back about whether we shouldn't do so (see the archives). I think
> > the major reason why we have not done it is fear of introducing
> > non-backwards-compatible behavior. Seems like 8.0 is exactly the right
> > version to consider doing that in.
>
> If my 2 cents are valid I agree with you, what I don't totally agree is why
> consider this bug as a *feature* in previous 8.0 version.
>
I don't think this was ever considered a feature (at least I never found
any evidence of that) but more the concern was that it was "expected
behavior" and changing that behavior might toss people into a loop who
were expecting it. I would certainly be in favor of changing it though
since I think it would make our function implementation a lot stronger,
and I don't really see any practical downsides to changing the behavior.
I agree with Tom that 8.0 does seem like the best time to make such a
change and I'd like to see this put up to a vote since I have _never_
seen anyone argue the case that the current functionality is
preferable. I'm sure some will argue that they don't want to make the
change now that we are already into beta, but istm if we can't make
determinations on version number jumps until we're in beta, we can't
also tie peoples hands completely on making changes once we're there.

Robert Treat
--
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

Robert Treat wrote:

> On Thu, 2004-08-26 at 04:23, Gaetano Mendola wrote:
>
>>Tom Lane wrote:
>>
>> > Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
>> >
>> >>I believe it sees the one that was valid in the snapshot as of the
>> >>beginning of the function.
>> >
>> >
>> > Actually, the problem is that it can see *both* that row and the updated
>> > row; it's a crapshoot which one will be returned by the SELECT INTO.
>>
>>Confirmed, if the last select is:
>>
>>select count(*) into a from test where id=1;
>>
>>this return 2. There is a space for a new bug considering that if the
>>table have the unique index on id that select must return 1.
>>
>> > The reason this can happen is that we're not doing SetQuerySnapshot
>> > between commands of a plpgsql function. There is discussion going way
>> > way back about whether we shouldn't do so (see the archives). I think
>> > the major reason why we have not done it is fear of introducing
>> > non-backwards-compatible behavior. Seems like 8.0 is exactly the right
>> > version to consider doing that in.
>>
>>If my 2 cents are valid I agree with you, what I don't totally agree is why
>>consider this bug as a *feature* in previous 8.0 version.
>>
>
> I don't think this was ever considered a feature (at least I never found
> any evidence of that) but more the concern was that it was "expected
> behavior" and changing that behavior might toss people into a loop who
> were expecting it.

Yes, I used the wrong expression is not a feature but a gotcha.
I fairly trust that someone is currently using this behaviour considering it
the good expected one.

Regards
Gaetano Mendola

On Thu, 2004-08-26 at 09:08, Gaetano Mendola wrote:
> Robert Treat wrote:
>
> > On Thu, 2004-08-26 at 04:23, Gaetano Mendola wrote:
> >
> >>Tom Lane wrote:
> >>
> >> > Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
> >> >
> >> >>I believe it sees the one that was valid in the snapshot as of the
> >> >>beginning of the function.
> >> >
> >> >
> >> > Actually, the problem is that it can see *both* that row and the updated
> >> > row; it's a crapshoot which one will be returned by the SELECT INTO.
> >>
> >>Confirmed, if the last select is:
> >>
> >>select count(*) into a from test where id=1;
> >>
> >>this return 2. There is a space for a new bug considering that if the
> >>table have the unique index on id that select must return 1.
> >>
> >> > The reason this can happen is that we're not doing SetQuerySnapshot
> >> > between commands of a plpgsql function. There is discussion going way
> >> > way back about whether we shouldn't do so (see the archives). I think
> >> > the major reason why we have not done it is fear of introducing
> >> > non-backwards-compatible behavior. Seems like 8.0 is exactly the right
> >> > version to consider doing that in.
> >>
> >>If my 2 cents are valid I agree with you, what I don't totally agree is why
> >>consider this bug as a *feature* in previous 8.0 version.
> >>
> >
> > I don't think this was ever considered a feature (at least I never found
> > any evidence of that) but more the concern was that it was "expected
> > behavior" and changing that behavior might toss people into a loop who
> > were expecting it.
>
> Yes, I used the wrong expression is not a feature but a gotcha.
> I fairly trust that someone is currently using this behaviour considering it
> the good expected one.
>

Really? I don't. I do suspect there are people using this behavior
considering it the bad, but expected, one. However if people really do
want this functionality to stay the same, let them speak up via a vote
on the subject.

Robert Treat
--
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Treat wrote:

| On Thu, 2004-08-26 at 09:08, Gaetano Mendola wrote:
|
|>Robert Treat wrote:
|>
|>
|>>On Thu, 2004-08-26 at 04:23, Gaetano Mendola wrote:
|>>
|>>
|>>>Tom Lane wrote:
|>>>
|>>>
|>>>>Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
|>>>>
|>>>>
|>>>>>I believe it sees the one that was valid in the snapshot as of the
|>>>>>beginning of the function.
|>>>>
|>>>>
|>>>>Actually, the problem is that it can see *both* that row and the updated
|>>>>row; it's a crapshoot which one will be returned by the SELECT INTO.
|>>>
|>>>Confirmed, if the last select is:
|>>>
|>>>select count(*) into a from test where id=1;
|>>>
|>>>this return 2. There is a space for a new bug considering that if the
|>>>table have the unique index on id that select must return 1.
|>>>
|>>>
|>>>>The reason this can happen is that we're not doing SetQuerySnapshot
|>>>>between commands of a plpgsql function. There is discussion going way
|>>>>way back about whether we shouldn't do so (see the archives). I think
|>>>>the major reason why we have not done it is fear of introducing
|>>>>non-backwards-compatible behavior. Seems like 8.0 is exactly the right
|>>>>version to consider doing that in.
|>>>
|>>>If my 2 cents are valid I agree with you, what I don't totally agree is why
|>>>consider this bug as a *feature* in previous 8.0 version.
|>>>
|>>
|>>I don't think this was ever considered a feature (at least I never found
|>>any evidence of that) but more the concern was that it was "expected
|>>behavior" and changing that behavior might toss people into a loop who
|>>were expecting it.
|>
|>Yes, I used the wrong expression is not a feature but a gotcha.
|>I fairly trust that someone is currently using this behaviour considering it
|>the good expected one.
|>
|
|
| Really? I don't. I do suspect there are people using this behavior
| considering it the bad, but expected, one. However if people really do
| want this functionality to stay the same, let them speak up via a vote
| on the subject.

A vote ? What do you expect from this question:

select count(*) from test where id=1;

shall return one or two given id primary key for the table "test"?

I hope that 100% will reply: one.

Regards
Gaetano Mendola

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (MingW32)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFBL3x17UpzwH2SGd4RArKIAKDU4rnhxt9GK7HL0Kjn0xzbPGHuvwCgkqtl
H1aLZ6j4CRnJD1S2cJ0Uf+A=
=yvUK
-----END PGP SIGNATURE-----

Robert Treat wrote:

> On Thu, 2004-08-26 at 09:08, Gaetano Mendola wrote:
>
>>Robert Treat wrote:
>>
>>
>>>On Thu, 2004-08-26 at 04:23, Gaetano Mendola wrote:
>>>
>>>
>>>>Tom Lane wrote:
>>>>
>>>>
>>>>>Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
>>>>>
>>>>>
>>>>>>I believe it sees the one that was valid in the snapshot as of the
>>>>>>beginning of the function.
>>>>>
>>>>>
>>>>>Actually, the problem is that it can see *both* that row and the updated
>>>>>row; it's a crapshoot which one will be returned by the SELECT INTO.
>>>>
>>>>Confirmed, if the last select is:
>>>>
>>>>select count(*) into a from test where id=1;
>>>>
>>>>this return 2. There is a space for a new bug considering that if the
>>>>table have the unique index on id that select must return 1.
>>>>
>>>>
>>>>>The reason this can happen is that we're not doing SetQuerySnapshot
>>>>>between commands of a plpgsql function. There is discussion going way
>>>>>way back about whether we shouldn't do so (see the archives). I think
>>>>>the major reason why we have not done it is fear of introducing
>>>>>non-backwards-compatible behavior. Seems like 8.0 is exactly the right
>>>>>version to consider doing that in.
>>>>
>>>>If my 2 cents are valid I agree with you, what I don't totally agree is why
>>>>consider this bug as a *feature* in previous 8.0 version.
>>>>
>>>
>>>I don't think this was ever considered a feature (at least I never found
>>>any evidence of that) but more the concern was that it was "expected
>>>behavior" and changing that behavior might toss people into a loop who
>>>were expecting it.
>>
>>Yes, I used the wrong expression is not a feature but a gotcha.
>>I fairly trust that someone is currently using this behaviour considering it
>>the good expected one.
>>
>
>
> Really? I don't.

Me neither but I'm realizing now that I wrote the opposite I would write :-)

Sorry for the noise, see my previous post.

Regards
Gaetano Mendola