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

  1. Align better test output regex with grammar in pg_dump TAP tests

  2. Tighten some regexes with proper character escaping in pg_dump TAP tests