From: Nikolay Shaplov <dhyan@nataraj.su>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org,
 Alvaro Herrera <alvherre@2ndquadrant.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Robert Haas <robertmhaas@gmail.com>,
 Nikita Glukhov <n.gluhov@postgrespro.ru>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH][PROPOSAL] Add enum releation option type
Date: Wed, 20 Feb 2019 09:38:03 +0300
Message-ID: <3859984.kzKLgvQqlp@x200m>
User-Agent: KMail/5.2.3 (Linux/4.9.0-8-amd64; KDE/5.28.0; x86_64; ; )
In-Reply-To: <20190220060832.GI15532@paquier.xyz>
References: <201901032112.xpqqwmumqml2@alvherre.pgsql>
 <1777585.CqHLqLDU7p@x200m> <20190220060832.GI15532@paquier.xyz>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

=D0=92 =D0=BF=D0=B8=D1=81=D1=8C=D0=BC=D0=B5 =D0=BE=D1=82 =D1=81=D1=80=D0=B5=
=D0=B4=D0=B0, 20 =D1=84=D0=B5=D0=B2=D1=80=D0=B0=D0=BB=D1=8F 2019 =D0=B3. 15=
:08:32 MSK =D0=BF=D0=BE=D0=BB=D1=8C=D0=B7=D0=BE=D0=B2=D0=B0=D1=82=D0=B5=D0=
=BB=D1=8C Michael=20
Paquier =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB:
> On Sun, Jan 06, 2019 at 06:28:11PM +0300, Nikolay Shaplov wrote:
> > Actually I am not satisfied with it too... That is why I started that
> > bigger reloptions refactoring work. So for now I would offer to keep
> > initialization as it was for other types: in initialize_reloptions  usi=
ng
> > [type]RelOpts[] arrays. And then I will offer patch that changes it for
> > all types and remove difference between biult-in reloptions and
> > reloptions in extensions.
> Should this be switched as waiting on author or just marked as
> returned with feedback then?
Actually I would prefer "Commited" ;-)

And speaking seriously... Anyway, this test module in src/test/modules shou=
ld=20
be a separate patch, as it would test that all types of options are properl=
y=20
reaching index extension, not only enum.

=46rom my point of view, it can be committed without any dependency with en=
um=20
reloption. Either we first commit this patch, and then that test module wil=
l=20
test that enum support, or first we commit that test moudule without testin=
g=20
enum support, and add tests into this enum patch.

I would prefer to commit enum first, and I would promise that I will add th=
us=20
module to the top of my dev TODO list. If it is not ok for you, then please=
=20
tell me about it and set this patch as "Waiting for author" and I will do t=
he=20
test patch first.

> > 2.5  May be this src/test/modules dummy index is subject to another pat=
ch.
> > So I will start working on it right now, but we will do this work not
> > dependent to any other patches. And just add there what we need when it
> > is ready. I would actually add there some code that checks all types of
> > options, because we actually never check that option value from relopto=
ns
> > successfully reaches extension code. I did this checks manually, when I
> > wrote that bigger reloption patch, but there is no facilities to do
> > regularly check is via test suit. Creating dummy index will create such
> > facilities.
>=20
> bloom is a user-facing extension, while src/test/modules are normally
> not installed (there is an exception for MSVC anyway..), so stressing
> add_enum_reloption in src/test/modules looks more appropriate to me,
> except if you can define an option which is useful for the user and is
> an enum.
Thank you for pointing me right direction. 've been waiting for it. Now I c=
an=20
go on :)) So let's it be src/test/modules module...