Re: WIP: Access method extendability

Hi!

On Fri, Apr 1, 2016 at 12:45 PM, Aleksander Alekseev <
a.alekseev@postgrespro.ru> wrote:

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

I've discussed this with Teodor privately.  Extra hash calculation could
cause performance regression.  He proposed to use own random generator
instead.  Implemented in attached version of patch.


> And a few more things.
>
> > +     memset(opaque, 0, sizeof(BloomPageOpaqueData));
> > +     opaque->maxoff = 0;
>


> This looks a bit redundant.
>

Fixed.


> > + for (my $i = 1; $i <= 10; $i++)
>
> More idiomatic Perl would be `for my $i (1..10)`.
>

Fixed.


> > +                     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?


Fixed with small code duplication which seems to be better than goto.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company