Re: WIP: Access method extendability - Mailing list pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, failed
Spec compliant:           not tested
Documentation:            not tested

There are currently no docs or unit tests. I suspect this patch is still WIP?

create-am.5.patch:
General notes:
Needs catversion bump.

IndexQualInfo and GenericCosts have been moved to src/include/utils/selfuncs.h.

METHOD becomes an unreserved keyword.

generic-xlog.5.patch:
generic_xlog.c: At least needs a bunch of explanatory comments, if not info in a README. Since I don't really
understandwhat the design here is my review is just cursory.
 

static memoryMove() - seems like an overly-generic function name to me...

writeCopyFlagment(), writeMoveFlagment(): s/Fl/Fr/?

bloom-control.5:
README:
+ CREATE INDEX bloomidx ON tbloom(i1,i2,i3) 
+        WITH (length=5, col1=2, col2=2, col3=4);
+ 
+ Here, we create bloom index with signature length 80 bits and attributes
+ i1, i2  mapped to 2 bits, attribute i3 - to 4 bits.

It's not clear to me where 80 bits is coming from...
bloom.h:
+ #define BITBYTE     (8)
ISTR seeing this defined somewhere in the Postgres headers; dunno if it's worth using that definition instead.

Testing:
I ran the SELECT INTO from the README, as well as CREATE INDEX bloomidx. I then ran

insert into tbloom select       (generate_series(1,1000)*random())::int as i1,
(generate_series(1,1000)*random())::intas i2,       (generate_series(1,1000)*random())::int as i3,
(generate_series(1,1000)*random())::intas i4,       (generate_series(1,1000)*random())::int as i5,
(generate_series(1,1000)*random())::intas i6,       (generate_series(1,1000)*random())::int as i7,
(generate_series(1,1000)*random())::intas i8,       (generate_series(1,1000)*random())::int as i9,
(generate_series(1,1000)*random())::intas i10,       (generate_series(1,1000)*random())::int as i11,
(generate_series(1,1000)*random())::intas i12,       (generate_series(1,1000)*random())::int as i13from
generate_series(1,999);

and kill -9'd the backend. After restart I did VACUUM VERBOSE without issue. I ran the INSERT INTO, kill -9 and VACUUM
VERBOSEsequence again. This time I got an assert:
 

TRAP: FailedAssertion("!(((bool) (((const void*)((ItemPointer) left) != ((void*)0)) && (((ItemPointer) left)->ip_posid
!=0))))", File: "vacuumlazy.c", Line: 1823)
 

That line is
lblk = ItemPointerGetBlockNumber((ItemPointer) left);

which does
AssertMacro(ItemPointerIsValid(pointer)), \

which is the assert that's failing.

I've squirreled this install away for now, in case you can't repro this failure.