Heikki Linnakangas wrote:
> On 02.02.2011 19:36, Kevin Grittner wrote:
>
>> Then there's one still lurking outside the new predicate* files,
>> in heapam.c. It's about 475 lines into the heap_update function
>> (25 lines from the bottom). In reviewing where we needed to
>> acquire predicate locks, this struck me as a place where we might
>> need to duplicate the predicate lock from one tuple to another,
>> but I wasn't entirely sure. I tried for a few hours one day to
>> construct a scenario which would show a serialization anomaly if I
>> didn't do this, and failed create one. If someone who understands
>> both the patch and heapam.c wants to look at this and offer an
>> opinion, I'd be most grateful. I'll take another look and see if I
>> can get my head around it better, but failing that, I'm
>> disinclined to either add locking I'm not sure we need or to
>> remove the comment that says we *might* need it there.
>
> Have you convinced yourself that there's nothing to do? If so, we
> should replace the todo comment with a comment explaining why.
It turns out that nagging doubt from my right-brain was on target.
Here's the simplest example I was able to construct of a false
negative due to the lack of some code there:
-- setup
create table t (id int not null, txt text) with (fillfactor=50);
insert into t (id) select x from (select * from generate_series(1, 1000000)) a(x);
alter table t add primary key (id);
-- session 1
-- T1
begin transaction isolation level serializable;
select * from t where id = 1000000;
-- session 2
-- T2
begin transaction isolation level serializable;
update t set txt = 'x' where id = 1000000;
commit;
-- T3
begin transaction isolation level serializable;
update t set txt = 'y' where id = 1000000;
select * from t where id = 500000;
-- session 3
-- T4
begin transaction isolation level serializable;
update t set txt = 'z' where id = 500000;
select * from t where id = 1;
commit;
-- session 2
commit;
-- session 1
update t set txt = 'a' where id = 1;
commit;
Based on visibility of work, there's a cycle in the apparent order of
execution:
T1 -> T2 -> T3 -> T4 -> T1
So now that I'm sure we actually do need code there, I'll add it.
And I'll add the new test to the isolation suite.
-Kevin