Re: Tighten up a few overly lax regexes in pg_dump's tap tests
Daniel Gustafsson <daniel@yesql.se>
From: Daniel Gustafsson <daniel@yesql.se>
To: Michael Paquier <michael@paquier.xyz>
Cc: David Rowley <david.rowley@2ndquadrant.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2019-02-06T14:41:20Z
Lists: pgsql-hackers
> On 6 Feb 2019, at 12:37, Michael Paquier <michael@paquier.xyz> wrote: > > On Wed, Feb 06, 2019 at 10:50:27AM +0100, Daniel Gustafsson wrote: >> I still think we should enforce one-or-more matching on the OWNER part as well, >> since matching zero would be a syntax error. There are more .* matches but >> I’ve only touched the ones which match SQL, since there is a defined grammar to >> rely on there. The attached patch does that on top of your commit. > > - regexp => qr/^COMMENT ON DATABASE postgres IS .*;/m, > + regexp => qr/^COMMENT ON DATABASE postgres IS .+;/m, > So... With what's on HEAD the current regex means that all these are > correct commands: > COMMENT ON DATABASE postgres is ; > COMMENT ON DATABASE postgres is foo; > COMMENT ON DATABASE postgres is foo; > And for the first one that's obviously wrong. > > So what you are suggesting is to actually make the first pattern > something to complain about, right? And ".+" this makes sure that at > least one character is present, while for ".*" it is fine to have zero > characters. What you are suggesting looks right if I understood that > right. Correct. One could argue that the regex is still suboptimal since “COMMENT ON DATABASE postgres IS ;” will be matched as well, but there I think the tradeoff for readability wins. cheers ./daniel
Commits
-
Align better test output regex with grammar in pg_dump TAP tests
- f339a998ffe6 12.0 landed
-
Tighten some regexes with proper character escaping in pg_dump TAP tests
- d07fb6810e51 12.0 landed