Re: Allow file inclusion in pg_hba and pg_ident files

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>
Subject: Re: Allow file inclusion in pg_hba and pg_ident files
Date: 2022-10-25 06:43:21
Message-ID: Y1eFicewsea5yGRj@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 24, 2022 at 04:03:03PM +0800, Julien Rouhaud wrote:
> It would also require to bring HbaLine->sourcefile. I'm afraid it would be
> weird to introduce such a refactoring in a separate commit just to pass a
> constant down multiple level of indirection, as all the macro will remain
> specific to either hba or ident anyway.

Putting my hands on it, I am not really afraid of doing that
independently. From what I can see, this is cutting 23kB worth of
diffs from 0002, reducing it from 94K to 71kB.

> I agree that there are quite a lot of s/XXXFileName/file_name/, but those
> aren't complicated, and keeping them in the same commit makes it easy to
> validate that none has been forgotten since the regression tests covering those
> messages are in that commit too.

Another advantage is that it minimizes the presence of the hardcoded
HbaFileName and IdentFileName in hba.c, which is one thing we are
trying to achieve here for the inclusion of more files. I found a bit
strange that IdentLine had no sourcefile, actually. We track the file
number but use it nowhere, and it seems to me that having more
symmetry between both would be a good thing.

So, the key of the logic is how we are going to organize the
tokenization of the HBA and ident lines through all the inclusions..
As far as I get it, tokenize_auth_file() is the root call and
tokenize_file_with_context() with its depth is able to work on each
individual file, and it can optionally recurse depending on what's
included. Why do you need to switch to the old context in
tokenize_file_with_context()? Could it be simpler to switch once to
linecxt outside of the internal routine?

It looks like GetDirConfFiles() is another piece that can be
refactored and reviewed on its own, as we use it in
ParseConfigDirectory()@guc.c.
--
Michael

Attachment Content-Type Size
v13-0001-Refactor-knowledge-of-origin-file-in-hba.c.patch text/x-diff 24.7 KB
v13-0002-Add-rule_number-mapping_number-to-the-pg_hba-pg_.patch text/x-diff 10.5 KB
v13-0003-Allow-file-inclusion-in-pg_hba-and-pg_ident-file.patch text/x-diff 70.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message wangw.fnst@fujitsu.com 2022-10-25 06:55:35 RE: Perform streaming logical transactions by background workers and parallel apply
Previous Message Peter Smith 2022-10-25 06:28:06 Re: Perform streaming logical transactions by background workers and parallel apply