Re: BRIN indexes - TRAP: BadArgument

Greg Stark wrote:
> On Tue, Nov 11, 2014 at 2:14 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)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.

Hmm. OpcInfo is also called in contexts other than scans, though, so
passing down scan keys into it seems wrong. Maybe we do need another
amproc that "initializes" the scan for the opclass, which would get
whatever got returned from opcinfo as well as scankeys. There you would
have the opportunity to run the hash and store it into the opaque.

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

As far as I understand, the scan keys don't change within any given
scan; if they do, the rescan AM method is called, at which point we
should reset whatever is cached about the previous scan.

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

Great, thanks.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services