Re: pgsql: Add TAP tests for pg_verify_checksums

Greetings,

* Andres Freund (andres@anarazel.de) wrote:
> On 2018-10-19 17:32:58 -0400, Stephen Frost wrote:
> > As you pointed out previously, the current code *doesn't* work, before
> > or after this change, and we clearly need to rework this to move things
> > into libpgcommon and also fix pg_basebackup.  Reverting this would at
> > least get us back to having similar code between this and pg_basebackup,
> > and then it'll be cleaner and clearer to have one patch which moves that
> > similar logic into libpgcommon and fixes the missing exceptions for the
> > EXEC_BACKEND case.
> > 
> > Keeping the patch doesn't do anything for the pg_basebackup case, and
> > confuses the issue by having these filename-pattern-whitelists which
> > weren't there before and that should be removed, imv.
> 
> The current state has pg_verify_checksum work on windows for casual
> usage, which includes the regression tests that previously were broken.
> Whereas reverting it has the advantage that a fixup diff would be a few
> lines smaller.  If you want to push a revert together with a fix, be my
> guest, but until that time it seems unhelpful to revert.

Clearly, pg_basebackup *doesn't* work though, so it's at best only half
a fix and only because our regression tests don't cover the
pg_basebackup case (which is a sad state of affairs, to say the least,
but an independent issue).

I'm not in any specific rush on this and I hope to hear Michael's
thoughts once he's had a chance to review our discussion; it's his
commit, after all.  Michael's initial patch is in the archives also, so
my suggestion would be to revert this one and then take Michael's prior
patch and add to it the fix of pg_basebackup, in the same manner, and
commit those changes together with additional comments to note that the
code exists in both places.

We could then work on moving the logic into libpgcommon, in master.

Thanks!

Stephen