Re: BRIN indexes - TRAP: BadArgument

Greg Stark <stark@mit.edu>

From: Greg Stark <stark@mit.edu>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Heikki Linnakangas <hlinnakangas@vmware.com>, Michael Paquier <michael.paquier@gmail.com>, David Rowley <dgrowleyml@gmail.com>, Jeff Janes <jeff.janes@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Erik Rijkers <er@xs4all.nl>, Emanuel Calvo <3manuek@esdebian.org>, Simon Riggs <simon@2ndquadrant.com>, Nicolas Barbier <nicolas.barbier@gmail.com>, Claudio Freire <klaussfreire@gmail.com>, Josh Berkus <josh@agliodbs.com>, Andres Freund <andres@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2014-11-11T08:14:20Z
Lists: pgsql-hackers
On Tue, Nov 11, 2014 at 2:14 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> I'm not sure why you say opaque is not associated with the specific
> scan.  Are you thinking we could reuse opaque for a future scan?  I
> think we could consider that opaque *is* the place to cache things such
> as the hashed value of the qual constants or whatever.

Oh. I guess this goes back to my original suggestion that the API docs
need to explain some sense of when OpcInfo is called. I didn't realize
it was tied to a specific scan. This does raise the question of why
the scan information isn't available in OpcInfo though. That would let
me build the hash value in a natural place instead of having to do it
lazily which I find significantly more awkward.

Is it possible for scan keys to change between calls for nested loop
joins or quirky SQL with volatile functions in the scan or anything? I
guess that would prevent the index scan from being used at all. But I
can be reassured the Opcinfo call will be called again when a cached
plan is reexecuted? Stable functions might have new values in a
subsequent execution even if the plan hasn't changed at all for
example.


> That's to test the Union procedure; if you look at the code, it's just
> used in assert-enabled builds.  Now that I think about it, perhaps this
> can turn out to be problematic for your bloom filter opclass.  I
> considered the idea of allowing the opclass to disable this testing
> procedure, but it isn't done (yet.)

No, it isn't a problem for my opclass other than performance, it was
quite helpful in turning up bugs early in fact. It was just a bit
confusing because I was trying to test things one by one and it turned
out the assertion checks meant a simple insert turned up bugs in Union
which I hadn't expected. But it seems perfectly sensible in an
assertion check.

-- 
greg


Commits

  1. Refactor per-page logic common to all redo routines to a new function.

  2. Reduce use of heavyweight locking inside hash AM.

  3. Scan the buffer pool just once, not once per fork, during relation drop.

  4. Major patch from Thomas Lockhart <Thomas.G.Lockhart@jpl.nasa.gov>