Re: Allow file inclusion in pg_hba and pg_ident files

On Tue, Mar 01, 2022 at 05:19:50PM +0800, Julien Rouhaud wrote:
> On Tue, Mar 01, 2022 at 04:45:48PM +0900, Michael Paquier wrote:
>> Hmm. The diffs of 0001 are really hard to read. Do you know why this
>> is happening? Is that because some code has been moved around?
>
> Yes, I followed the file convention to put the static functions first and then
> the exposed functions, and git-diff makes a terrible mess out of it :(

I'd like to think that not doing such a thing would be more helpful in
this case. As the diffs show, anyone is going to have a hard time to
figure out if there are any differences in any of those routines, and
if these are the origin of a different problem. A second thing is
that this is going to make back-patching unnecessarily harder.

> There's no functional change apart from exposing some functions and moving some
> in another file, so I though it's still ok to keep some consistency. There
> isn't much changes backpatched in that file, so it shouldn't create more
> maintenance burden than simply splitting the file.

A lot of files do that already. History clarity matters most IMO.

> As I mentioned in my initial email, I intentionally didn't add any test in the
> patchset yet, except the exact same coverage for the new view as there's for
> pg_hba_file_rules. Ideally I'd like to add tests only once, to cover both 002
> and 0003. But I don't want to waste time for that right now, especially since
> no one seems to be interested in 0003.

But that would be helpful for 0002. I think that we should have a bit
more coverage in this area. pg_hba_file_rules could gain in coverage,
additionally, but this is unrelated to what you are proposing here..

>> Does this pass
>> on Windows where pg_regress sets some mappings for SSPI when creating
>> one or more roles?
>
> According to CI and cfbot yes. E.g.
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/37/3558.
> Note that the failed runs are the warning I mentioned for mingw32 and the POC
> 0004, which is now fixed.

Interesting, I would not have expected that. I may poke at that more
seriously.
--
Michael