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> (raw)
In-Reply-To: <20170903094543.kkqdbdjuxwa5z6ji@alvherre.pgsql>
References: <20170903094543.kkqdbdjuxwa5z6ji@alvherre.pgsql>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
В письме от 3 сентября 2017 11:45:43 пользователь Alvaro Herrera написал:
> > I've just made sure that patch is still applyable to the current master.
> >
> > And I am still waiting for reviews :-)
>
> 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 for
it https://www.postgresql.org/message-id/2615372.orqtEn8VGB%40x200m (I will
add that thread to commitfest as soon as I understand what test should be left
there)
> I also noticed that your patch adds an error message that didn't exist
> previously,
>
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("col%i should not be grater than length",
> i)));
>
> this seems a bit troublesome, because of the "col%i" thing
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
code again, and explain what exactly I've done. (I did it a year ago and now
need some time to recall)
> The new checks around toast table creation look like they belong to a
> completely independent patch also ...
This sounds reasonable too. Will do it, it is possible, as far as I remember.
> the error message there goes against message style guidelines also.
Oh... these style guidelines... will pay attention to it too...
--
Do code for fun. Can do it for money (Perl & C/C++ ~10h/week)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: dhyan@nataraj.su, alvherre@alvh.no-ip.org, 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
In-Reply-To: <4134457.STgLtIlFYK@x200m>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox