Allow regex operations to be terminated early by query cancel requests.

The regex code didn't have any provision for query cancel; which is
unsurprising given its non-Postgres origin, but still problematic since
some operations can take a long time.  Introduce a callback function to
check for a pending query cancel or session termination request, and
call it in a couple of strategic spots where we can make the regex code
exit with an error indicator.

If we ever actually split out the regex code as a standalone library,
some additional work will be needed to let the cancel callback function
be specified externally to the library.  But that's straightforward
(certainly so by comparison to putting the locale-dependent character
classification logic on a similar arms-length basis), and there seems
no need to do it right now.

A bigger issue is that there may be more places than these two where
we need to check for cancels.  We can always add more checks later,
now that the infrastructure is in place.

Since there are known examples of not-terribly-long regexes that can
lock up a backend for a long time, back-patch to all supported branches.
I have hopes of fixing the known performance problems later, but adding
query cancel ability seems like a good idea even if they were all fixed.

diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index c99a1f13d6aec17c3b568536136e8de7524a7581..e0e32344ca45d5a6b861c66373704863b71fe4d8 100644 (file)
--- a/src/backend/regex/regc_nfa.c
+++ b/src/backend/regex/regc_nfa.c
@@ -174,11 +174,23 @@ newstate(struct nfa * nfa)
 {
        struct state *s;
 
+       /*
+        * This is a handy place to check for operation cancel during regex
+        * compilation, since no code path will go very long without making a new
+        * state.
+        */
+       if (CANCEL_REQUESTED(nfa->v->re))
+       {
+               NERR(REG_CANCEL);
+               return NULL;
+       }
+
        if (TooManyStates(nfa))
        {
                NERR(REG_ETOOBIG);
                return NULL;
        }
+
        if (nfa->free != NULL)
        {
                s = nfa->free;

diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index e5e5ee86bf6ad7ca0560c9f10f4beace0390f8a9..e121fd9a85edcda76ad05e46e5901c4fdb4d826b 100644 (file)
--- a/src/backend/regex/regcomp.c
+++ b/src/backend/regex/regcomp.c
@@ -34,6 +34,8 @@
 
 #include "regex/regguts.h"
 
+#include "miscadmin.h"                 /* needed by rcancelrequested() */
+
 /*
  * forward declarations, up here so forward datatypes etc. are defined early
  */
@@ -67,6 +69,7 @@ static long nfanode(struct vars *, struct subre *, FILE *);
 static int     newlacon(struct vars *, struct state *, struct state *, int);
 static void freelacons(struct subre *, int);
 static void rfree(regex_t *);
+static int     rcancelrequested(void);
 
 #ifdef REG_DEBUG
 static void dump(regex_t *, FILE *);
@@ -274,6 +277,7 @@ struct vars
 /* static function list */
 static struct fns functions = {
        rfree,                                          /* regfree insides */
+       rcancelrequested                        /* check for cancel request */
 };
 
 
@@ -1869,6 +1873,22 @@ rfree(regex_t *re)
        }
 }
 
+/*
+ * rcancelrequested - check for external request to cancel regex operation
+ *
+ * Return nonzero to fail the operation with error code REG_CANCEL,
+ * zero to keep going
+ *
+ * The current implementation is Postgres-specific.  If we ever get around
+ * to splitting the regex code out as a standalone library, there will need
+ * to be some API to let applications define a callback function for this.
+ */
+static int
+rcancelrequested(void)
+{
+       return InterruptPending && (QueryCancelPending || ProcDiePending);
+}
+
 #ifdef REG_DEBUG
 
 /*

diff --git a/src/backend/regex/regexec.c b/src/backend/regex/regexec.c
index 16cdd89757148f591d39f62ddd840f7d80aca8f5..59a7da49b970f49b21611395118da3d116e7478f 100644 (file)
--- a/src/backend/regex/regexec.c
+++ b/src/backend/regex/regexec.c
@@ -710,6 +710,10 @@ cdissect(struct vars * v,
        assert(t != NULL);
        MDEBUG(("cdissect %ld-%ld %c\n", LOFF(begin), LOFF(end), t->op));
 
+       /* handy place to check for operation cancel */
+       if (CANCEL_REQUESTED(v->re))
+               return REG_CANCEL;
+
        switch (t->op)
        {
                case '=':                               /* terminal node */

diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.c
index ef17ebb169ee6a81d3dafb448f4e639517ec9c45..8b48d34fdbcec693ad36dccd35951618cce0e13e 100644 (file)
--- a/src/backend/utils/adt/regexp.c
+++ b/src/backend/utils/adt/regexp.c
@@ -31,6 +31,7 @@
 
 #include "catalog/pg_type.h"
 #include "funcapi.h"
+#include "miscadmin.h"
 #include "regex/regex.h"
 #include "utils/builtins.h"
 #include "utils/guc.h"
@@ -183,6 +184,15 @@ RE_compile_and_cache(text *text_re, int cflags)
        if (regcomp_result != REG_OKAY)
        {
                /* re didn't compile (no need for pg_regfree, if so) */
+
+               /*
+                * Here and in other places in this file, do CHECK_FOR_INTERRUPTS
+                * before reporting a regex error.      This is so that if the regex
+                * library aborts and returns REG_CANCEL, we don't print an error
+                * message that implies the regex was invalid.
+                */
+               CHECK_FOR_INTERRUPTS();
+
                pg_regerror(regcomp_result, &re_temp.cre_re, errMsg, sizeof(errMsg));
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
@@ -262,6 +272,7 @@ RE_wchar_execute(regex_t *re, pg_wchar *data, int data_len,
        if (regexec_result != REG_OKAY && regexec_result != REG_NOMATCH)
        {
                /* re failed??? */
+               CHECK_FOR_INTERRUPTS();
                pg_regerror(regexec_result, re, errMsg, sizeof(errMsg));
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
@@ -1194,6 +1205,7 @@ regexp_fixed_prefix(text *text_re, bool case_insensitive,
 
                default:
                        /* re failed??? */
+                       CHECK_FOR_INTERRUPTS();
                        pg_regerror(re_result, re, errMsg, sizeof(errMsg));
                        ereport(ERROR,
                                        (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index f64d46169839e5561e1dc30ee704a81b82de574f..f14a9bd3fb255e4d9d47d5a880bdcc9760b67c23 100644 (file)
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2785,6 +2785,7 @@ replace_text_regexp(text *src_text, void *regexp,
                {
                        char            errMsg[100];
 
+                       CHECK_FOR_INTERRUPTS();
                        pg_regerror(regexec_result, re, errMsg, sizeof(errMsg));
                        ereport(ERROR,
                                        (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),

diff --git a/src/include/regex/regerrs.h b/src/include/regex/regerrs.h
index 15da324b66041534117169c7e6c3f54a889333a7..956b56b50ee305764bc780e3917f8bcb4c83a385 100644 (file)
--- a/src/include/regex/regerrs.h
+++ b/src/include/regex/regerrs.h
@@ -81,3 +81,7 @@
 {
        REG_ECOLORS, "REG_ECOLORS", "too many colors"
 },
+
+{
+       REG_CANCEL, "REG_CANCEL", "operation cancelled"
+},

diff --git a/src/include/regex/regex.h b/src/include/regex/regex.h
index adaf1a168d7d49812137e2c58c341d0edc7b1180..0680d4bcfd597b378f78b8f7c80764d7750c04d7 100644 (file)
--- a/src/include/regex/regex.h
+++ b/src/include/regex/regex.h
@@ -153,6 +153,7 @@ typedef struct
 #define REG_BADOPT     18                      /* invalid embedded option */
 #define REG_ETOOBIG 19                 /* nfa has too many states */
 #define REG_ECOLORS 20                 /* too many colors */
+#define REG_CANCEL     21                      /* operation cancelled */
 /* two specials for debugging and testing */
 #define REG_ATOI       101                     /* convert error-code name to number */
 #define REG_ITOA       102                     /* convert error-code number to name */

diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h
index 9cd0ef9fdf6d07fa30d65c0729c68241a68fb1a4..1a0a8bd7cd45e627eeaf18f89374ee88e8e91497 100644 (file)
--- a/src/include/regex/regguts.h
+++ b/src/include/regex/regguts.h
@@ -403,8 +403,11 @@ struct subre
 struct fns
 {
        void            FUNCPTR(free, (regex_t *));
+       int                     FUNCPTR(cancel_requested, (void));
 };
 
+#define CANCEL_REQUESTED(re)  \
+       ((*((struct fns *) (re)->re_fns)->cancel_requested) ())
 
 
 /*