Re: WIP: Access method extendability

Aleksander Alekseev <a.alekseev@postgrespro.ru>

From: Aleksander Alekseev <a.alekseev@postgrespro.ru>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Teodor Sigaev <teodor@sigaev.ru>, Petr Jelinek <petr@2ndquadrant.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, David Steele <david@pgmasters.net>, Michael Paquier <michael.paquier@gmail.com>, Jim Nasby <Jim.Nasby@bluetreble.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2016-04-01T09:45:08Z
Lists: pgsql-hackers
Hello, Alexander

> Hi!
> 
> New revision of patches is attached.

Code looks much better now, thanks. Still I believe it could be improved.

I don't think that using srand() / rand() in signValue procedure the
way you did is such a good idea. You create a side affect (changing
current randseed) which could cause problems in some cases. And there
is no real need for that. For instance you could use following formula
instead:

hash(attno || hashVal || j)

And a few more things.

> + 	memset(opaque, 0, sizeof(BloomPageOpaqueData));
> + 	opaque->maxoff = 0;

This looks a bit redundant.

> + for (my $i = 1; $i <= 10; $i++)

More idiomatic Perl would be `for my $i (1..10)`.

> + 			UnlockReleaseBuffer(buffer);
> + 			ReleaseBuffer(metaBuffer);
> + 			goto away;

In general I don't have anything against goto. But are you sure that
using it here is really justified?

-- 
Best regards,
Aleksander Alekseev
http://eax.me/


Commits

  1. Support CREATE ACCESS METHOD

  2. Restructure index access method API to hide most of it at the C level.