Re: WIP: BRIN multi-range indexes - Mailing list pgsql-hackers
From | Alexander Korotkov |
---|---|
Subject | Re: WIP: BRIN multi-range indexes |
Date | |
Msg-id | CAPpHfduRd9sZy5M9NYwbvgSLMt4+X104+7J1pvUimq+w66n54A@mail.gmail.com Whole thread Raw |
In response to | Re: WIP: BRIN multi-range indexes (Tomas Vondra <tomas.vondra@2ndquadrant.com>) |
Responses |
Re: WIP: BRIN multi-range indexes
(Tomas Vondra <tomas.vondra@2ndquadrant.com>)
|
List | pgsql-hackers |
On Sun, Apr 5, 2020 at 6:53 PM Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote: > On Sun, Apr 05, 2020 at 06:29:15PM +0300, Alexander Korotkov wrote: > >On Thu, Apr 2, 2020 at 5:29 AM Tomas Vondra > ><tomas.vondra@2ndquadrant.com> wrote: > >> On Sun, Dec 01, 2019 at 10:55:02AM +0900, Michael Paquier wrote: > >> >On Thu, Sep 26, 2019 at 09:01:48PM +0200, Tomas Vondra wrote: > >> >> Yeah, the opclass params patches got broken by 773df883e adding enum > >> >> reloptions. The breakage is somewhat extensive so I'll leave it up to > >> >> Nikita to fix it in [1]. Until that happens, apply the patches on > >> >> top of caba97a9d9 for review. > >> > > >> >This has been close to two months now, so I have the patch as RwF. > >> >Feel free to update if you think that's incorrect. > >> > >> I see the opclass parameters patch got committed a couple days ago, so > >> I've rebased the patch series on top of it. The pach was marked RwF > >> since 2019-11, so I'll add it to the next CF. > > > >I think this patchset was marked RwF mainly because slow progress on > >opclass parameters. Now we got opclass parameters committed, and I > >think this patchset is in a pretty good shape. Moreover, opclass > >parameters patch comes with very small examples. This patchset would > >be great showcase for opclass parameters. > > > >I'd like to give this patchset a chance for v13. I'm going to make > >another pass trough this patchset. If I wouldn't find serious issues, > >I'm going to commit it. Any objections? > > > > I'm an author of the patchset and I'd love to see it committed, but I > think that might be a bit too rushed and unfair (considering it was not > included in the current CF at all). > > I think the code is correct and I'm not aware of any bugs, but I'm not > sure there was sufficient discussion about things like costing, choosing > parameter values (e.g. number of values in the multi-minmax or bloom > filter parameters). Ok! > That being said, I think the first couple of patches (that modify how > BRIN deals with multi-key scans and IS NULL clauses) are simple enough > and non-controversial, so maybe we could get 0001-0003 committed, and > leave the bloom/multi-minmax opclasses for v14. Regarding 0001-0003 I've couple of notes: 1) They should revise BRIN extensibility documentation section. 2) I think 0002 and 0003 should be merged. NULL ScanKeys should be still passed to consistent function when oi_regular_nulls == false. 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. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql-hackers by date: