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
-
Support CREATE ACCESS METHOD
- 473b93287040 9.6.0 landed
-
Restructure index access method API to hide most of it at the C level.
- 65c5fcd353a8 9.6.0 cited