From: Nikolay Shaplov <dhyan@nataraj.su>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@postgresql.org, Andres Freund <andres@anarazel.de>
Subject: Re: [PATCH] Move all am-related reloption code into
 src/backend/access/[am-name] and get rid of relopt_kind for custom AM
Date: Tue, 05 Sep 2017 12:55:40 +0300
Message-ID: <4134457.STgLtIlFYK@x200m>
User-Agent: KMail/4.14.1 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; )
In-Reply-To: <20170903094543.kkqdbdjuxwa5z6ji@alvherre.pgsql>
References: <20170903094543.kkqdbdjuxwa5z6ji@alvherre.pgsql>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

=D0=92 =D0=BF=D0=B8=D1=81=D1=8C=D0=BC=D0=B5 =D0=BE=D1=82 3 =D1=81=D0=B5=D0=
=BD=D1=82=D1=8F=D0=B1=D1=80=D1=8F 2017 11:45:43 =D0=BF=D0=BE=D0=BB=D1=8C=D0=
=B7=D0=BE=D0=B2=D0=B0=D1=82=D0=B5=D0=BB=D1=8C Alvaro Herrera =D0=BD=D0=B0=
=D0=BF=D0=B8=D1=81=D0=B0=D0=BB:

> > I've just made sure that patch is still applyable to the current master.
> >=20
> > And I am still waiting for reviews :-)
>=20
> I see that this patch adds a few tests for reloptions, for instance in
> bloom.  I think we should split this in at least two commits, one that
> adds those tests before the functionality change, so that they can be
> committed in advance, verify that the buildfarm likes it with the
> current code, and verify the coverage.
This sounds as a good idea. I created such patch and started a new thread f=
or=20
it https://www.postgresql.org/message-id/2615372.orqtEn8VGB%40x200m (I will=
=20
add that thread to commitfest as soon as I understand what test should be l=
eft=20
there)

> I also noticed that your patch adds an error message that didn't exist
> previously,
>=20
> +               ereport(ERROR,
> +                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> +                      errmsg("col%i should not be grater than length",
> i)));
>=20
> this seems a bit troublesome, because of the "col%i" thing=20
What the problem with col%i ?

> and also because of the typo.
Oh... I always had bad marks for all natural languages in school :-( ;-)

> I wonder if this means you've changed the way sanity checks work here.
This should not be like this (I hope). I will attentively look at this part=
 of=20
code again, and explain what exactly I've done. (I did it a year ago and no=
w=20
need some time to recall)

> The new checks around toast table creation look like they belong to a
> completely independent patch also ...=20
This sounds reasonable too. Will do it, it is possible, as far as I remembe=
r.

> the error message there goes against message style guidelines also.
Oh... these style guidelines... will pay attention to it too...

--=20
Do code for fun. Can do it for money (Perl & C/C++ ~10h/week)


--=20
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers