On 8/25/19 4:39 AM, Tom Lane wrote:
Ian Barwick <ian.barw...@2ndquadrant.com> writes:
On 7/17/19 5:34 PM, Kyotaro Horiguchi wrote:> Hello.
I don't think this is new to 12.
No, though I'm not sure how much this would be seen as a bugfix
and how far back it would be sensible to patch.
I think this is worth considering as a bugfix; although I'm afraid
we can't change the signature of ParseConfigFile/ParseConfigFp in
released branches, since extensions could possibly be using those.
That limits what we can do --- but it's still possible to detect
direct recursion, which seems like enough to produce a nice error
message in typical cases.
Makes sense.
I concur with Kyotaro-san that disallow-empty-include-directives.v1.patch
seems a bit brute-force, but where I would put the checks is in
ParseConfigFile and ParseConfigDirectory.
Also, I don't agree with the goals of prevent-disallowed-includes.patch.
I'm utterly not on board with breaking use of "include" in extension
files, for instance; while that may not be documented, it works fine,
and maybe somebody out there is relying on it.
I couldn't for the life of me think of any reason for using it.
But if there's undocumented functionality we think someone might
be using, shouldn't that be documented somewhere, if only as a note
in the code to prevent its accidental removal at a later date?
Likewise, while "include"
in pg.auto.conf is not really considered supported, I don't see the
point of going out of our way to break the historical behavior.
The amusing thing about that of course is that the include directive
will disappear the next time ALTER SYSTEM is run and the values from
the included file will appear in pg.auto.conf, which may cause some
headscratching. But I guess hasn't been an actual real-world
issue so far.
That leads me to propose the attached simplified patch. While I haven't
actually tried, I'm pretty sure this should back-patch without trouble.
Ah, I see it's been applied already, thanks!
Regards
Ian Barwick
--
Ian Barwick https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
