Re: WIP: Access method extendability

Поиск
Список
Период
Сортировка
От Alexander Korotkov
Тема Re: WIP: Access method extendability
Дата
Msg-id CAPpHfdvMv=r0mooBfTAjQ1pzYD4UAF_suQozSi5=WrbMmWptJA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: WIP: Access method extendability  (Jim Nasby <Jim.Nasby@BlueTreble.com>)
Ответы Re: WIP: Access method extendability  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Список pgsql-hackers
Hi!

Next version of patch is attached.

On Tue, Feb 2, 2016 at 4:09 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
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?
 
I hope to exit WIP state soon...

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

Yes. I think catversion bump is committer responsibility and shouldn't be included into patch.
 
IndexQualInfo and GenericCosts have been moved to src/include/utils/selfuncs.h.
 
Yes, they have been moved in order to be accessed by custom index access method.

METHOD becomes an unreserved keyword.

Seems to be inevitable if we want CREATE ACCESS METHOD command.
 
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 understand what the design here is my review is just cursory.

Make detailed explanation of API in generic_xlog.h. Could be moved into README if needed.
 
static memoryMove() - seems like an overly-generic function name to me...

I've simplified generic xlog to just per byte comparison without support of memory move. Now it would be much easier to commit. Support of memory move could be added in the separate patch though. 
 
writeCopyFlagment(), writeMoveFlagment(): s/Fl/Fr/?

Fixed. 

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...

length is measure in uint16...
For now, it's documented.
 
bloom.h:
+ #define BITBYTE       (8)
ISTR seeing this defined somewhere in the Postgres headers; dunno if it's worth using that definition instead.

Got rid of this. Using BITS_PER_BYTE now.
 
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())::int as i2,
        (generate_series(1,1000)*random())::int as i3,
        (generate_series(1,1000)*random())::int as i4,
        (generate_series(1,1000)*random())::int as i5,
        (generate_series(1,1000)*random())::int as i6,
        (generate_series(1,1000)*random())::int as i7,
        (generate_series(1,1000)*random())::int as i8,
        (generate_series(1,1000)*random())::int as i9,
        (generate_series(1,1000)*random())::int as i10,
        (generate_series(1,1000)*random())::int as i11,
        (generate_series(1,1000)*random())::int as i12,
        (generate_series(1,1000)*random())::int as i13
 from 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 VERBOSE sequence 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.
Should be fixed.

General notes about current version of patch:
* A lot of comments added.
* bloom documentation is moved from README to SGML with a lot of addons and cleanup.
* Memory move support in generic xlog is removed. Now it's much more simple and clean.
* Tests for CREATE ACCESS METHOD added. For now, it creates a mirror of GiST access method.
* Syntax for CREATE ACCESS METHOD is changed. For now, it's "CREATE ACCESS METHOD amname TYPE INDEX HANDLER handler;" in respect of parallel work on sequential access methods. New access method attribute added: amtype.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
 
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: "Daniel Verite"
Дата:
Сообщение: Re: [patch] Proposal for \crosstabview in psql
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Small PATCH: check of 2 Perl modules