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
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) |