MIME-Version: 1.0
References: <20180123200537.gfx5hxcp5k5hktpe@alvherre.pgsql>
 <821c4c1d-592e-8916-e323-04607b11777e@2ndquadrant.com>
 <c32f9f46-e0ca-0c02-45ec-718dd381f4be@2ndquadrant.com>
 <104E8BB8-35FF-4AA8-9871-6AFF04E4CAF3@gmail.com>
 <0afde2fc-e160-004f-8fe2-c65302ac3760@2ndquadrant.com>
 <22674.1517874015@sss.pgh.pa.us>
 <3e924a4c-6ca0-cc10-aebb-5004fc002800@2ndquadrant.com>
 <c2523129-a970-97f2-4705-a3b65e21b32f@2ndquadrant.com>
 <20180302035538.gkqgtcv6kmn2cklw@alap3.anarazel.de>
 <18526.1519963686@sss.pgh.pa.us>
 <1c1c1ced-1f79-a0ee-e5cd-858ca3eb9d74@2ndquadrant.com>
 <CAPpHfdsUSu+tEo6Kaj_iXvrhzhGOZ2pkYEn_aWj9b4M2vHGEhg@mail.gmail.com>
 <acfc8aa3-1cbf-b9de-77ba-556ddd84a094@2ndquadrant.com>
 <CAPpHfduD_30yZ4H8CE9BHHedci-qUpuhX941mkV0jR7Nd3AJWA@mail.gmail.com>
 <49cb668f-d6f9-3493-681d-7d40b715ef64@postgrespro.ru>
 <90be37f9-07ea-0f90-4bc6-6bdb68f4f190@2ndquadrant.com>
 <CAPpHfdsC8Y8+2ZvDM6m_7E6ZN7b0ev4Z2LvHd6tJCCsTsnGVrg@mail.gmail.com>
 <bb3d9479-b8f7-a1be-587d-830f4d41b047@2ndquadrant.com>
In-Reply-To: <bb3d9479-b8f7-a1be-587d-830f4d41b047@2ndquadrant.com>
From: Alexander Korotkov <a.korotkov@postgrespro.ru>
Date: Wed, 13 Mar 2019 14:09:19 +0300
Message-ID: 
 <CAPpHfdtUc0X0gO2dzm65sOg_38DiDkV-_fWDatJ1Bqi-FmhH9w@mail.gmail.com>
Subject: Re: WIP: BRIN multi-range indexes
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>, Tom Lane <tgl@sss.pgh.pa.us>,
	Andres Freund <andres@anarazel.de>, Mark Dilger <hornschnorter@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Mar 13, 2019 at 12:52 PM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> On 3/13/19 9:15 AM, Alexander Korotkov wrote:
> > On Tue, Mar 12, 2019 at 8:15 PM Tomas Vondra
> > <tomas.vondra@2ndquadrant.com> wrote:
> >>> 0001. Pass all keys to BRIN consistent function at once.
> >>>
> >>> I think that changing the signature of consistent function is bad, because then
> >>> the authors of existing BRIN opclasses will need to maintain two variants of
> >>> the function for different version of PosgreSQL.  Moreover, we can easily
> >>> distinguish two variants by the number of parameters.  So I returned back a
> >>> call to old 3-argument variant of consistent() in bringetbitmap().  Also I
> >>> fixed brinvalidate() adding support for new 4-argument variant, and fixed
> >>> catalog entries for brin_minmax_consistent() and brin_inclusion_consistent()
> >>> which remained 3-argument.  And also I removed unneeded indentation shift in
> >>> these two functions, which makes it difficult to compare changes, by extracting
> >>> subroutines minmax_consistent_key() and inclusion_consistent_key().
> >>>
> >>
> >> Hmmm. I admit I rather dislike functions that change the signature based
> >> on the number of arguments, for some reason. But maybe it's better than
> >> changing the consistent function. Not sure.
> >
> > I also kind of dislike signature change based on the number of
> > arguments.  But it's still good to let extensions use old interface if
> > needed.  What do you think about invention new consistent method, so
> > that extension can implement one of them?  We did similar thing for
> > GIN (bistate consistent vs tristate consistent).
> >
>
> Possibly. The other annoyance of course is that to support the current
> consistent method we'll have to keep all the code I guess :-(

Yes, because incompatible change of opclass support function signature
is the thing we never did before.  We have to add new optional
arguments to GiST functions, but that was compatible change.

If we do incompatible change of opclass interface, it becomes unclear
to do pg_upgrade with extension installed.  Imagine, if we don't
require function signature to match, we could easily get segfault
because of extension incompatibility.  If we do require function
signature to match, extension upgrade would become complex.  It would
be required to not only adjust C-code, but also write some custom
script, which changes opclass (and users would have to run this script
manually?).

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