Re: Extensions, this time with a patch
Itagaki Takahiro <itagaki.takahiro@gmail.com>
From: Itagaki Takahiro <itagaki.takahiro@gmail.com>
To: Dimitri Fontaine <dimitri@2ndquadrant.fr>
Cc: Alvaro Herrera <alvherre@commandprompt.com>, David Fetter <david@fetter.org>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2010-11-25T05:44:24Z
Lists: pgsql-hackers
On Thu, Nov 25, 2010 at 05:02, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
> Something like the attached, version 5 of the patch? I've been using the
> function name ParseConfigFp because the internal parameter was called fp
> in the previous function body. I suppose that could easily be changed at
> commit time if necessary.
Thanks. I'll move the patch to Ready for Committer.
Here is a list of items for committers, including only cosmetic changes.
- Comments in recovery.conf.sample needs to be adjusted.
# (The quotes around the value are NOT optional, but the "=" is.)
It seems to be the only description about quotes are not omissible before.
- We might not need to check the result of ParseConfigFp() because
it always raises FATAL on errors.
- We could remove the unused argument "calling_file" in ParseConfigFp().
- I feel "struct name_value_pair" and "ConfigNameValuePair" a bit
redundant names. I'd prefer something like ConfigVariable.
- "fp" might be a better name than FILE *fd. There are 4 usages in xlog.c.
> Extensions will need the support for custom_variable_classes as it is
> done now, and as you say, the recovery will just error out. You have to
> clean your recovery.conf file then try again (I just tried and confirm).
>
> I personally don't see any harm to have the features in all currently
> known uses-cases, and I don't see any point in walking an extra mile
> here to remove a feature in some cases.
Sure. We will leave them.
--
Itagaki Takahiro