Re: [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM

В письме от 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)