> On 6 Feb 2019, at 12:37, Michael Paquier <mich...@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
