Re: Tighten up a few overly lax regexes in pg_dump's tap tests

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: David Fetter <david(at)fetter(dot)org>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Tighten up a few overly lax regexes in pg_dump's tap tests
Date: 2019-02-07 20:17:39
Message-ID: 81D64405-4CC3-4CE4-BBF2-6860532C735F@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 7 Feb 2019, at 18:20, David Fetter <david(at)fetter(dot)org> wrote:
>
> On Thu, Feb 07, 2019 at 10:10:32AM +0900, Michael Paquier wrote:
>> On Wed, Feb 06, 2019 at 03:41:20PM +0100, Daniel Gustafsson wrote:
>>> 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.
>>
>> Okay, that looks like an improvement anyway, so committed after going
>> over the tests for similar problems, and there was one for CREATE
>> DATABASE and DROP ROLE. It is possible to have a regex which tells as
>> at least one non-whitespace character, but from what I can see these
>> don't really improve the readability.
>
> Are you talking about \w+, or [^[:space:]]+, [^[:blank:]]+, or…?

Personally I feel that expanding these test regexes to catch more low-risk
bugs, at the cost of readability, is a slippery slope towards implementing a
SQL parser in regexes. That was my $0.02 for not attempting to make these
bulletproof.

cheers ./daniel

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2019-02-07 20:23:43 Re: Synchronize with imath upstream
Previous Message Tom Lane 2019-02-07 20:08:28 Re: PG_RE_THROW is mandatory (was Re: jsonpath)