On Fri, Mar 8, 2024 at 1:22 AM Robert Haas <robertmh...@gmail.com> wrote:
> On Thu, Mar 7, 2024 at 9:16 AM Robert Haas <robertmh...@gmail.com> wrote: > > It could. I just thought this was clearer. I agree that it's a bit > > long, but I don't think this is worth bikeshedding very much. If at a > > later time somebody feels strongly that it needs to be changed, so be > > it. Right now, getting on with the business at hand is more important, > > IMHO. > > Here's a new version of the patch set, rebased over my version of 0001 > and with various other corrections: > > * Tidy up grammar in documentation. > * In manifest_process_version, the test checked whether the manifest > version == 1, but the comment talked about versions >= 2. Make the > comment match the code. > * In load_backup_manifest, avoid changing the existing placement of a > variable declaration. > * Rename verify_system_identifier to verify_control_file because if we > were verifying multiple things about the control file we'd still want > to only read it one. > * Tweak the coding of verify_backup_file and verify_control_file to > avoid repeated path construction. > * Remove saw_system_identifier_field. This looks like it's trying to > enforce a rule that the system identifier must immediately follow the > version, but we don't insist on anything like that for files or wal > ranges, so there seems to be no reason to do it here. > * Remove bogus "unrecognized top-level field" test from > 005_bad_manifest.pl. The JSON included here doesn't include any > unrecognized top-level field, so the fact that we were getting that > error message was wrong. After removing saw_system_identifier_field, > we no longer get the wrong error message any more, so the test started > failing. > * Remove "expected system identifier" test from 005_bad_manifest.pl. > This was basically a test that saw_system_identifier_field was > working. > * Header comment adjustment for > json_manifest_finalize_system_identifier. The last sentence was > cut-and-pasted from somewhere that it made sense to here, where it > doesn't. There's only ever one system identifier. > > Thank you for the improvement. The caller of verify_control_file() has the full path of the control file that can pass it and avoid recomputing. With this change, it doesn't really need verifier_context argument -- only the manifest's system identifier is enough along with the control file path. Did the same in the attached delta patch for v11-0002 patch, please have a look, thanks. Regards, Amul
v11-0002-delta.patch
Description: Binary data
