Re: Extensions, this time with a patch

Itagaki Takahiro <itagaki.takahiro@gmail.com> writes:
> RECOVERY_COMMAND_FILE is opened twice in the patch. The first time
> is for checking the existence, and the second time is for parsing.
> Instead of the repeat, how about adding FILE* version of parser?
> It will be also called from ParseConfigFile() as a sub routine.
>
>   bool ParseConfigFd(FILE *fd, const char *config_file, int depth, ...)

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.

> BTW, the parser supports "include" and "custom_variable_classes"
> not only for postgresql.conf but also for all files. Is it an
> intended behavior?  I think they are harmless, so we don't have
> to change the codes; "include" might be useful even in recovery.conf,
> and "custom_variable_classes" will be "unrecognized recovery
> parameter" error after all.

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.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support