> I looked at that aspect of the patch specifically a few weeks back while > checking for possible issues with Hot Standby. IIRC the patch is fairly > careful with locking and uses Exclusive locks extensively throughout. I > looked at both the theory and the implementation. Unless Gianni changed > something in that regard recently, I don't understand that comment at > all. Probably need to provide specific examples of your concerns. > The major thing there is to get the modifications right. There is no much sense in reviewing "wrong" code against "locking issues".
I wish to focus on the performance aspect of the patch, however, it turned out there are major issues with functionality: the index stores wrong tids inside :( I really would love to fix that issue and have a chance to validate the performance. Unfortunately, I have spent more than a day with almost void success. I have two testcases for which the index fails to get the correct result: Testcase 1 (I guess there is a conflict between _bitmap_formitem and mergewords): Basically I create a table with all the rows equal to 1 besides 19-th, which is 0. create table t1 as select case when i=19 then 0 else 1 end as i from generate_series(1,20) as s(i) create index t1ix on t1 using bitmap (i) where i = 0; set enable_seqscan=off; select ctid,i From t1 where i=0; -- no rows selected. Debug shows index suggests ctid==(0,35) instead of (0,19). 35==16+16+3. Testcase 2 create table t2 as select i, 0 j from generate_series(1,1000) as s(i); update t2 set j=1 where i in (5, 230) create index t2ix on t2 using bitmap(j) where j=1; set enable_seqscan=off; select ctid, i, j from t2 where j=1; -- no rows selected. Debug shows index suggests ctids==(0,97) and (0,98) instead of (4,97) and (4,98) -- it loses page number somewhere on the way. Both testcases reveal defects in index creation. Regards, Vladimir Sitnikov
