On Thu, Oct 27, 2022 at 12:26:25PM +0800, Julien Rouhaud wrote: > On Thu, Oct 27, 2022 at 12:08:31PM +0900, Michael Paquier wrote: >> >> Putting things afresh, there are two different things here (sorry I >> need to see that typed ;p): >> 1) How do we want to check reliably the loading of the HBA and ident >> files on errors? > > I guess you meant the failure to load HBA / ident files containing invalid > data?
Yeah. >> Hmm. And what if we just gave up on the checks for error patterns in >> pg_hba_file_rules? One part that I was thinking about when typing this part yesterday is that an EXEC_BACKEND build should work in non-WIN32 in TAP even if pg_ident.conf cannot be loaded, but I forgot entirely about the part where we need a user mapping for the SSPI authentication on WIN32, as set by pg_regress. > We discussed this problem in the past (1), and my understanding was that > detecting a -DEXEC_BACKEND/Win32 build and skipping those tests in that case > would be an acceptable solution to make sure there's at least some coverage. > The proposed patch adds such an approach, making sure that the failure is due > to an invalid HBA file. If you changed you mind I can remove that part, but > again I'd like to be sure of what you exactly want before starting to rewrite > stuff. I am still not completely sure what's the best way to do things here, so let's do the following: let's keep the patch the way you think is better for now (I may change my opinion on that but I'll hack that by myself anyway). Using what you have as a base, could you split the test and have it in its simplest to ease irs review? It would be able to stress the buildfarm with a first version of the test and see how it goes from there, and it is useful by itself IMO as HEAD has zero coverage for this area. > I'm not sure what you mean here. The patch does check for all the errors > looking at LOG lines and CONTEXT lines, but to make the regexp easier it > doesn't try to make sure that each CONTEXT line is immediately following the > expected LOG line. Hmm. Perhaps we'd better make sure that the LOG/CONTEXT link is checked? The context includes the line number while a generic sentence, and the LOG provides all the details of the error happening. > That's why the errors are divided in 2 steps: a first step with a single error > using some inclusion, so we can validate that the CONTEXT line is entirely > correct (wrt. line number and such), and then every possible error pattern > where we assume that the CONTEXT line are still following their LOG entry if > they're found. It also has the knowledge of which errors adds a CONTEXT line > and which don't. And that's done twice, for HBA and ident. Okay, so you do check the relationship between both, after all. > The part 3 is just concatenating everything back, for HBA and ident. So > long-term maintenance shouldn't get any harder as there won't be any need for > more steps. We can just keep appending stuff in the 2nd step and all the > tests > should run as expected. Hmm. Okay. -- Michael
signature.asc
Description: PGP signature
