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

On Fri, 8 Feb 2019 at 13:04, Michael Paquier wrote: > > On Thu, Feb 07, 2019 at 03:33:54PM +0100, Daniel Gustafsson wrote: > > Correct. The idea was to maintain readability while making the regex a bit > > better, without any claims to make it perfect. > > Agreed with your position. I see no poi

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

On Thu, Feb 07, 2019 at 03:33:54PM +0100, Daniel Gustafsson wrote: > Correct. The idea was to maintain readability while making the regex a bit > better, without any claims to make it perfect. Agreed with your position. I see no point is making all the expressions unreadable for little gain. Wh

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

> On 7 Feb 2019, at 18:20, David Fetter 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 I

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

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

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

> On 7 Feb 2019, at 13:55, Tels wrote: > On Wed, February 6, 2019 8:10 pm, 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 matche

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

Moin, On Wed, February 6, 2019 8:10 pm, 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 >> tra

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

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 any

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

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

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

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 define

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

On Wed, 6 Feb 2019 at 21:39, Michael Paquier wrote: > And done after checking the whole set. Thanks for pushing. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services

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

> On 6 Feb 2019, at 09:39, Michael Paquier wrote: > And done after checking the whole set. 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

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

On Tue, Feb 05, 2019 at 04:26:18PM +0900, Michael Paquier wrote: > On Tue, Feb 05, 2019 at 05:46:55PM +1300, David Rowley wrote: >> I did leave a couple untouched as there was quite a bit of escaping >> going on already. I didn't think switching between \Q and \E would >> have made those ones any m

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

> On 5 Feb 2019, at 06:55, David G. Johnston wrote: > > On Monday, February 4, 2019, David Rowley > wrote: > On Tue, 5 Feb 2019 at 01:12, Daniel Gustafsson > wrote: > > We may also want to use the + metacharacter instead of * in a few

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

On Tue, Feb 05, 2019 at 05:46:55PM +1300, David Rowley wrote: > I did leave a couple untouched as there was quite a bit of escaping > going on already. I didn't think switching between \Q and \E would > have made those ones any more pleasing to the eye. -qr/^ALTER TABLE dump_test.test_table EN

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

On Monday, February 4, 2019, David Rowley wrote: > On Tue, 5 Feb 2019 at 01:12, Daniel Gustafsson wrote: > > We may also want to use the + metacharacter instead of * in a few > places, since > > the intent is to always match something, where matching nothing should be > > considered an error: >

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

On Tue, 5 Feb 2019 at 01:12, Daniel Gustafsson wrote: > We may also want to use the + metacharacter instead of * in a few places, > since > the intent is to always match something, where matching nothing should be > considered an error: > > - qr/^ALTER TEXT SEARCH DICTIONARY dump_

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

On Tue, 5 Feb 2019 at 14:41, Michael Paquier wrote: > Instead of the approach you are > proposing, perhaps it would make sense to extend this way of doing > things then? For example some tests with CREATE CONVERSION do so. It > looks much more portable than having to escape every dot. I'm not

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

On Tue, Feb 05, 2019 at 01:50:50PM +1300, David Rowley wrote: > I think I've done all the ones that use \E. Those \Q ones don't need > any escaping. I assume that's the ones you've found. I didn't do an > exhaustive check though. Oh, good point. I didn't know that anything between \Q and \E are

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

On Tue, 5 Feb 2019 at 13:15, Michael Paquier wrote: > Some tests are missing the update, and it seems to me that > tightening things up is a good thing, still we ought to do it > consistently. Some places missing the update: > - ALTER OPERATOR FAMILY > - ALTER OPERATOR CLASS > - ALTER SEQUENCE >

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

On Mon, Feb 04, 2019 at 01:12:48PM +0100, Daniel Gustafsson wrote: > +1 for tightening it up, and the patch looks good to me. > > We may also want to use the + metacharacter instead of * in a few places, > since > the intent is to always match something, where matching nothing should be > conside

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

> On 4 Feb 2019, at 11:54, David Rowley wrote: > > There are a few regexes in pg_dump's tap tests that are neglecting to > escape the dot in "schema.table" type expressions. This could result > in the test passing when it shouldn't. It seems worth putting that > right. +1 for tightening it up,