Re: WIP: BRIN multi-range indexes

Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> schrieb am Fr., 10. Juli 2020,
14:09:

> On Fri, Jul 10, 2020 at 06:01:58PM +0900, Masahiko Sawada wrote:
> >On Fri, 3 Jul 2020 at 09:58, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
> wrote:
> >>
> >> On Sun, Apr 05, 2020 at 08:01:50PM +0300, Alexander Korotkov wrote:
> >> >On Sun, Apr 5, 2020 at 8:00 PM Tomas Vondra
> >> ><tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
> >> ...
> >> >> >
> >> >> >Assuming we're not going to get 0001-0003 into v13, I'm not so
> >> >> >inclined to rush on these three as well. But you're willing to
> commit
> >> >> >them, you can count round of review on me.
> >> >> >
> >> >>
> >> >> I have no intention to get 0001-0003 committed. I think those changes
> >> >> are beneficial on their own, but the primary reason was to support
> the
> >> >> new opclasses (which require those changes). And those parts are not
> >> >> going to make it into v13 ...
> >> >
> >> >OK, no problem.
> >> >Let's do this for v14.
> >> >
> >>
> >> Hi Alexander,
> >>
> >> Are you still interested in reviewing those patches? I'll take a look at
> >> 0001-0003 to check that your previous feedback was addressed. Do you
> >> have any comments about 0004 / 0005, which I think are the more
> >> interesting parts of this series?
> >>
> >>
> >> Attached is a rebased version - I realized I forgot to include 0005 in
> >> the last update, for some reason.
> >>
> >
> >I've done a quick test with this patch set. I wonder if we can improve
> >brin_page_items() SQL function in pageinspect as well. Currently,
> >brin_page_items() is hard-coded to support only normal brin indexes.
> >When we pass brin-bloom or brin-multi-range to that function the
> >binary values are shown in 'value' column but it seems not helpful for
> >users. For instance, here is an output of brin_page_items() with a
> >brin-multi-range index:
> >
> >postgres(1:12801)=# select * from brin_page_items(get_raw_page('mul',
> >2), 'mul');
> >-[ RECORD 1
> ]----------------------------------------------------------------------------------------------------------------------
>
> >-----------------------------------------------------------------------------------------------------------------------------------
> >----------------------------
> >itemoffset | 1
> >blknum | 0
> >attnum | 1
> >allnulls | f
> >hasnulls | f
> >placeholder | f
> >value |
> {\x010000001b0000002000000001000000e5700000e6700000e7700000e8700000e9700000ea700000eb700000ec700000ed700000ee700000ef
>
> >700000f0700000f1700000f2700000f3700000f4700000f5700000f6700000f7700000f8700000f9700000fa700000fb700000fc700000fd700000fe700000ff700
> >00000710000}
> >
>
> Hmm. I'm not sure we can do much better, without making the function
> much more complicated. I mean, even with regular BRIN indexes we don't
> really know if the value is plain min/max, right?
>
You can be sure with the next node. The value is in can be false positiv.
The value is out is clear. You can detect the change between in and out.

>
>
> >Also, I got an assertion failure when setting false_positive_rate
> reloption:
> >
> >postgres(1:12448)=# create index blm on t using brin (c int4_bloom_ops
> >(false_positive_rate = 1));
> >TRAP: FailedAssertion("(false_positive_rate > 0) &&
> >(false_positive_rate < 1.0)", File: "brin_bloom.c", Line: 300)
> >
> >I'll look at the code in depth and let you know if I find a problem.
> >
>
> Yeah, the assert should say (f_p_r <= 1.0).
>
> But I'm not convinced we should allow values up to 1.0, really. The
> f_p_r is the fraction of the table that will get matched always, so 1.0
> would mean we get to scan the whole table. Seems kinda pointless. So
> maybe we should cap it to something like 0.1 or so, but I agree the
> value seems kinda arbitrary.
>
>
> regards
>
> --
> Tomas Vondra http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>