Re: Extensions, this time with a patch

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