RE: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
> I see postgres 7.1.1 is out now. Was the fix for this > problem included in the new release? I fear it will be in 7.2 only. > On Thursday 29 March 2001 20:02, Philip Warner wrote: > > At 19:14 29/03/01 -0800, Mikheev, Vadim wrote: > > >> >Reported problem is caused by bug (only one tuple > version must be > > >> >returned by SELECT) and this is way to fix it. > > >> > > >> I assume this is not possible in 7.1? > > > > > >Just looked in heapam.c - I can fix it in two hours. > > >The question is - should we do this now? > > >Comments? > > > > It's a bug; how confident are you of the fix? > > ---(end of > broadcast)--- > TIP 4: Don't 'kill -9' the postmaster > ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
Re: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
On Thursday 29 March 2001 22:15, Tom Lane wrote: > > Just looked in heapam.c - I can fix it in two hours. > > The question is - should we do this now? > > This scares the hell out of me. > > I do NOT think we should be making quick-hack changes in fundamental > system semantics at this point of the release cycle. Although I'm the one who is being bit by this bug, I tend to agree. > The problem went unnoticed for two full release cycles I first reported the problem on 25 September 2000, on the pgsql-sql list, message subject "SQL functions not locking properly?" I was using 7.0.2 at the time. Also, I seem to remember that a problem of this nature bit me in 6.5.x as well. > it can wait another cycle for a fix that has been considered, reviewed, > and tested. Let's not risk making things worse by releasing a new > behavior we might find out is also wrong. Good point. How long is the next cycle likely to take? Forest ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
RE: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
> > > I doubt if it's a bug of SELECT. Well what > > > 'concurrent UPDATE then SELECT FOR UPDATE + > > > SELECT' return ? > > > > I'm going to add additional check to heapgettup and > > heap_fetch: > > SELECT seems to be able to return a different result > from that of preceding SELECT FOR UPDATE even after > applying your change. Oh, you're right. Well, if we really want that SELECT returns the same result as SELECT FOR UPDATE *in functions* (out of functions results are already same) then we have to add some modifications to fix proposed: 1. If newer version of visible tuple T is marked for update by us *before* query began then do not return T. 2. If tuple T1 is *not visible* because of it was inserted by concurrent committed TX then check if it's marked for update by current TX *before* query began and return *this* tuple version if yes. This will be in accordance with standard which requires us return committed (whenever) rows in READ COMMITTED mode. In fact, in this mode our SELECTs provide higher isolation than required by standard returning rows committed *before* query began. Why? Because of SELECT doesn't lock rows and the same row may be visited by SELECT in join queries many times - so we have to be protected against concurrent updates. SELECT FOR UPDATE protects us BUT if query itself calls some functions which updates queried table then currently we may lose information that tuple was marked for update before query began - so updating tuple inserted by concurrent committed TX and marked for update by us we would have to save its t_cmax in t_cmin (and either add new flag to t_infomask or don't turn OFF HEAP_MARKED_FOR_UPDATE in this case). This is not what I would like to do in 7.1 > SELECT doesn't seem guilty but the result is far > from intuitive. I think that SELECT is guilty. At least returning two versions of the same row! (One that could be fixed easy). Vadim ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
"Mikheev, Vadim" wrote: > > > > >> I assume this is not possible in 7.1? > > > > > > > >Just looked in heapam.c - I can fix it in two hours. > > > >The question is - should we do this now? > > > >Comments? > > > > > > It's a bug; how confident are you of the fix? > > 95% -:) > > > I doubt if it's a bug of SELECT. Well what > > 'concurrent UPDATE then SELECT FOR UPDATE + > > SELECT' return ? > > I'm going to add additional check to heapgettup and > heap_fetch: > SELECT seems to be able to return a different result from that of preceding SELECT FOR UPDATE even after applying your change. SELECT doesn't seem guilty but the result is far from intuitive. It seems impossoble for all queires inside such a function to use a common snapshot. regards, Hiroshi Inoue ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
RE: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
> > >> I assume this is not possible in 7.1?
> > >
> > >Just looked in heapam.c - I can fix it in two hours.
> > >The question is - should we do this now?
> > >Comments?
> >
> > It's a bug; how confident are you of the fix?
95% -:)
> I doubt if it's a bug of SELECT. Well what
> 'concurrent UPDATE then SELECT FOR UPDATE +
> SELECT' return ?
I'm going to add additional check to heapgettup and
heap_fetch:
HeapTupleSatisfies(T) is TRUE:
IF XactIsoLevel is READ_COMMITTED
and snapshot != SnapshotDirty
and !(T->t_data->t_infomask & HEAP_XMAX_INVALID)
and T->t_data->t_infomask & HEAP_XMAX_COMMITTED
and T->t_self != T->t_data->t_ctid
{
FOR ( ; ; )
{
fetch tuple->t_data->t_ctid tuple
IF t_infomask & (HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE)
break; -- and return T
IF t_infomask & HEAP_XMAX_COMMITTED
{
IF t_self != ctid -- updated
continue;
break; -- deleted, return T
}
-- uncommitted update/delete
IF t_xmax != CurrentTransactionID
break; -- and return T
-- changed by current TX!
IF changed *BEFORE* this query began
{
-- DELETE + SELECT: nothing to return
-- UPDATE + SELECT: newer tuple version
-- will be/was returned by query
return NULL;
}
continue;
}
}
Vadim
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/users-lounge/docs/faq.html
Re: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
Philip Warner wrote: > > At 19:14 29/03/01 -0800, Mikheev, Vadim wrote: > >> >Reported problem is caused by bug (only one tuple version must be > >> >returned by SELECT) and this is way to fix it. > >> > > >> > >> I assume this is not possible in 7.1? > > > >Just looked in heapam.c - I can fix it in two hours. > >The question is - should we do this now? > >Comments? > > It's a bug; how confident are you of the fix? > I doubt if it's a bug of SELECT. Well what 'concurrent UPDATE then SELECT FOR UPDATE + SELECT' return ? regards, Hiroshi Inoue ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
RE: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
> >Reported problem is caused by bug (only one tuple version must be > >returned by SELECT) and this is way to fix it. > > > > I assume this is not possible in 7.1? Just looked in heapam.c - I can fix it in two hours. The question is - should we do this now? Comments? Vadim ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
"Mikheev, Vadim" <[EMAIL PROTECTED]> writes: > Hm, you're right: > http://www.postgresql.org/devel-corner/docs/postgres/xact-read-committed.html > "Read Committed is the default isolation level in Postgres. When > a transaction runs on this isolation level, a SELECT query sees only > data committed before the transaction began..." > ^^ > Must be "committed before the *query* began" as it was in 6.5 docs. Will fix this shortly... regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
RE: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
> Looking at the docs, I see that 'SERIALIZABLE' has the same visibility > rules as 'READ COMMITTED', which is very confusing. I expect Hm, you're right: http://www.postgresql.org/devel-corner/docs/postgres/xact-read-committed.htm l "Read Committed is the default isolation level in Postgres. When a transaction runs on this isolation level, a SELECT query sees only data committed before the transaction began..." ^^ Must be "committed before the *query* began" as it was in 6.5 docs. Any way to fix it before release? Vadim ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
RE: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
> > I don't think that we dare try to make any basic changes in > > MVCC for 7.1 at this late hour, so Forest is going to have > > to live with that answer for awhile. But I would like to see > > a cleaner answer in future releases. > > Is it the MVCC's restriction that each query inside a function > must use the same snapshot ? No. MVCC restricts what is visible to query itself. Current function' behaviour is like Oracle' one. Strictly speaking queries inside function don't use the same snapshot - they see changes made by other queries of this function. Should we allow them see changes made by other transactions? I'm not sure. Maybe by special means like CREATE SNAPSHOT S; SELECT FROM foo WITH SNAPSHOT S; ? For this particular case - concurrent UPDATE then UPDATE/DELETE + SELECT - there is simple solution: meeting tuple updated by concurrent *committed* transaction SELECT (in READ COMMITTED mode) should look in newer tuple versions. If some of newer tuples is invalidated (updated/deleted) by *this* transaction and this invalidation is *visible* to SELECT (older CommandId) then old tuple version must not be returned (newer tuple version will be returned of course). Reported problem is caused by bug (only one tuple version must be returned by SELECT) and this is way to fix it. But note that for the case of concurrent DELETE then INSERT + SELECT two tuples will be returned anyway and I don't think that this is bug. > > As I've opined before, the whole EvalPlanQual mechanism > > strikes me as essentially bogus in any case... > > How would you change it ? UPDATE/SELECT FOR UPDATE have to > SELECT/UPDATE the latest tuples. I don't think of any simple > way for 'SELECT FOR UPDATE' to have the same visibility as > simple SELECT. Yes, I also don't understand what's wrong with EvalPlanQual. Vadim ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] Re: [SQL] possible row locking bug in 7.0.3 & 7.1
Tom Lane wrote: > > Philip Warner <[EMAIL PROTECTED]> writes: > >> The workaround for Forest is to make the final SELECT be a SELECT FOR > >> UPDATE, so that it's playing by the same rules as the earlier commands. > > > Eek. Does this seem good to you? > > I did call it a workaround ;-) > > I don't think that we dare try to make any basic changes in MVCC for 7.1 > at this late hour, so Forest is going to have to live with that answer > for awhile. But I would like to see a cleaner answer in future > releases. Is it the MVCC's restriction that each query inside a function must use the same snapshot ? > As I've opined before, the whole EvalPlanQual mechanism > strikes me as essentially bogus in any case... > How would you change it ? UPDATE/SELECT FOR UPDATE have to SELECT/UPDATE the latest tuples. I don't think of any simple way for 'SELECT FOR UPDATE' to have the same visibility as simple SELECT. regards, Hiroshi Inoue ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
