Horton

Re: Another regexp performance improvement: skip useless paren-captures

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Andrew Dunstan <andrew@dunslane.net>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Joel Jacobson <joel@compiler.org>
Date: 2021-08-09T22:23:36Z
Lists: pgsql-hackers 
I wrote:
> Hmmm ... yeah, I see it too.  This points up something I'd wondered
> about before, which is whether the code that "cancels everything"
> after detecting {0} is really OK.  It throws away the outer subre
> *and children* without worrying about what might be inside, and
> here we see that that's not good enough --- there's still a v->subs
> pointer to the first capturing paren set, which we just deleted,
> so that the \1 later on messes up.  I'm not sure why the back
> branches are managing not to crash, but that might just be a memory
> management artifact.

... yeah, it is.  For me, this variant hits the assertion in all
branches:

regression=# select regexp_split_to_array('', '((.)){0}(\2){0}');
server closed the connection unexpectedly

So that's a pre-existing (and very long-standing) bug.  I'm not
sure if it has any serious impact in non-assert builds though.
Failure to clean out some disconnected arcs probably has no
real effect on the regex's behavior later.

			regards, tom lane

Commits

  1. Fix regexp misbehavior with capturing parens inside "{0}".

  2. Let regexp_replace() make use of REG_NOSUB when feasible.

  3. Fix bogus assertion in BootstrapModeMain().

  4. Avoid determining regexp subexpression matches, when possible.

  5. Check the size in COPY_POINTER_FIELD

  6. Make regexp engine's backref-related compilation state more bulletproof.