| From: | Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru> |
|---|---|
| To: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
| Cc: | Teodor Sigaev <teodor(at)sigaev(dot)ru>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: WIP: Access method extendability |
| Date: | 2016-04-01 09:45:08 |
| Message-ID: | 20160401124508.41c052c4@fujitsu |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2016-04-01 09:45:44 | Re: Logical Decoding - Execute join query |
| Previous Message | hari.prasath | 2016-04-01 09:39:59 | Logical Decoding - Execute join query |