Re: pgsql: Add TAP tests for pg_verify_checksums
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Michael Banck <michael.banck@credativ.de>, Andres Freund <andres@anarazel.de>, Postgres hackers <pgsql-hackers@postgresql.org>, Daniel Gustafsson <daniel@yesql.se>, Magnus Hagander <magnus@hagander.net>
Date: 2018-10-14T22:41:38Z
Lists: pgsql-hackers
Attachments
- checksum-whitelist-v3.patch (text/x-diff) patch v3
On Sun, Oct 14, 2018 at 10:24:55AM -0400, Andrew Dunstan wrote: > [snip] Thanks a lot for the review, Andrew! > This code now seems redundant: > > if (strcmp(fn, ".") == 0 || > strcmp(fn, "..") == 0) > return true; Indeed. I have renamed skipfile() to isRelFileName on the way, which looks better in the context. > I would probably reverse the order of these two tests. It might not make any > difference, assuming fn is never an empty string, but it seems more logical > to me. > > + /* good to go if only digits */ > + if (fn[pos] == '\0') > + return false; > + /* leave if no digits */ > + if (pos == 0) > + return true; No objections to that. Done. > It also looks to me like the check for a segment number doesn't ensure > there is at least one digit, so "123." might pass, but I could be > wrong. In any case, there isn't a test for that, and there probably > should be. You are right here. This name pattern has been failing. Fixed in the attached. I have added "123_" and "123_." as extra patterns to check. What do you think about the updated version attached? -- Michael
Commits
-
Fix issues with TAP tests of pg_verify_checksums
- 9685d7383ab4 12.0 landed
-
Use whitelist to choose files scanned with pg_verify_checksums
- cc7f27eae888 11.1 landed
- d55241af7056 12.0 landed
-
Add TAP tests for pg_verify_checksums
- b34e84f160a4 12.0 cited
-
Work around Msys weakness in Testlib.pm's command_like()
- efd7f8e36553 10.0 cited