Fix regexport.c to behave sanely with lookaround constraints.

regexport.c thought it could just ignore LACON arcs, but the correct
behavior is to treat them as satisfiable while consuming zero input
(rather reminiscently of commit 9f1e642d5).  Otherwise, the emitted
simplified-NFA representation may contain no paths leading from initial
to final state, which unsurprisingly confuses pg_trgm, as seen in
bug #14623 from Jeff Janes.

Since regexport's output representation has no concept of an arc that
consumes zero input, recurse internally to find the next normal arc(s)
after any LACON transitions.  We'd be forced into changing that
representation if a LACON could be the last arc reaching the final
state, but fortunately the regex library never builds NFAs with such
a configuration, so there always is a next normal arc.

Back-patch to 9.3 where this logic was introduced.

Discussion: https://postgr.es/m/20170413180503.25948.94871@wrigleys.postgresql.org

diff --git a/contrib/pg_trgm/expected/pg_trgm.out b/contrib/pg_trgm/expected/pg_trgm.out
index df6c1ff5139fc29cb34d9d6f17f001634314e9c5..6b2fa7eb4969087648f6cfffeb33965952d801af 100644 (file)
--- a/contrib/pg_trgm/expected/pg_trgm.out
+++ b/contrib/pg_trgm/expected/pg_trgm.out
@@ -3684,6 +3684,12 @@ select * from test2 where t ~ '  z foo';
    z foo bar
 (1 row)
 
+select * from test2 where t ~ 'qua(?!foo)';
+   t   
+-------
+ quark
+(1 row)
+
 drop index test2_idx_gin;
 create index test2_idx_gist on test2 using gist (t gist_trgm_ops);
 set enable_seqscan=off;
@@ -3864,6 +3870,12 @@ select * from test2 where t ~ '  z foo';
    z foo bar
 (1 row)
 
+select * from test2 where t ~ 'qua(?!foo)';
+   t   
+-------
+ quark
+(1 row)
+
 -- Check similarity threshold (bug #14202)
 CREATE TEMP TABLE restaurants (city text);
 INSERT INTO restaurants SELECT 'Warsaw' FROM generate_series(1, 10000);

diff --git a/contrib/pg_trgm/sql/pg_trgm.sql b/contrib/pg_trgm/sql/pg_trgm.sql
index 806f3eb7049c1b18c8506f62be720c9caf467af7..2619ad5ee62b65002055c9c4d8742ec36d7fabfc 100644 (file)
--- a/contrib/pg_trgm/sql/pg_trgm.sql
+++ b/contrib/pg_trgm/sql/pg_trgm.sql
@@ -86,7 +86,9 @@ select * from test2 where t ~ 'z foo bar';
 select * from test2 where t ~ ' z foo bar';
 select * from test2 where t ~ '  z foo bar';
 select * from test2 where t ~ '  z foo';
+select * from test2 where t ~ 'qua(?!foo)';
 drop index test2_idx_gin;
+
 create index test2_idx_gist on test2 using gist (t gist_trgm_ops);
 set enable_seqscan=off;
 explain (costs off)
@@ -121,6 +123,7 @@ select * from test2 where t ~ 'z foo bar';
 select * from test2 where t ~ ' z foo bar';
 select * from test2 where t ~ '  z foo bar';
 select * from test2 where t ~ '  z foo';
+select * from test2 where t ~ 'qua(?!foo)';
 
 -- Check similarity threshold (bug #14202)
 

diff --git a/src/backend/regex/regexport.c b/src/backend/regex/regexport.c
index 3856ff873f26f13a9e0781ffcb7a5beb226a4864..371c1f9da67d2cf2407f30bbb6934c95145d4026 100644 (file)
--- a/src/backend/regex/regexport.c
+++ b/src/backend/regex/regexport.c
@@ -6,8 +6,8 @@
  * In this implementation, the NFA defines a necessary but not sufficient
  * condition for a string to match the regex: that is, there can be strings
  * that match the NFA but don't match the full regex, but not vice versa.
- * Thus, for example, it is okay for the functions below to ignore lookaround
- * constraints, which merely constrain the string some more.
+ * Thus, for example, it is okay for the functions below to treat lookaround
+ * constraints as no-ops, since they merely constrain the string some more.
  *
  * Notice that these functions return info into caller-provided arrays
  * rather than doing their own malloc's.  This simplifies the APIs by
@@ -72,29 +72,78 @@ pg_reg_getfinalstate(const regex_t *regex)
 }
 
 /*
- * Get number of outgoing NFA arcs of state number "st".
+ * pg_reg_getnumoutarcs() and pg_reg_getoutarcs() mask the existence of LACON
+ * arcs from the caller, treating any LACON as being automatically satisfied.
+ * Since the output representation does not support arcs that consume no
+ * character when traversed, we have to recursively traverse LACON arcs here,
+ * and report whatever normal arcs are reachable by traversing LACON arcs.
+ * Note that this wouldn't work if it were possible to reach the final state
+ * via LACON traversal, but the regex library never builds NFAs that have
+ * LACON arcs leading directly to the final state.  (This is because the
+ * regex executor is designed to consume one character beyond the nominal
+ * match end --- possibly an EOS indicator --- so there is always a set of
+ * ordinary arcs leading to the final state.)
  *
- * Note: LACON arcs are ignored, both here and in pg_reg_getoutarcs().
+ * traverse_lacons is a recursive subroutine used by both exported functions
+ * to count and then emit the reachable regular arcs.  *arcs_count is
+ * incremented by the number of reachable arcs, and as many as will fit in
+ * arcs_len (possibly 0) are emitted into arcs[].
+ */
+static void
+traverse_lacons(struct cnfa * cnfa, int st,
+                               int *arcs_count,
+                               regex_arc_t *arcs, int arcs_len)
+{
+       struct carc *ca;
+
+       /*
+        * Since this function recurses, it could theoretically be driven to stack
+        * overflow.  In practice, this is mostly useful to backstop against a
+        * failure of the regex compiler to remove a loop of LACON arcs.
+        */
+       check_stack_depth();
+
+       for (ca = cnfa->states[st]; ca->co != COLORLESS; ca++)
+       {
+               if (ca->co < cnfa->ncolors)
+               {
+                       /* Ordinary arc, so count and possibly emit it */
+                       int                     ndx = (*arcs_count)++;
+
+                       if (ndx < arcs_len)
+                       {
+                               arcs[ndx].co = ca->co;
+                               arcs[ndx].to = ca->to;
+                       }
+               }
+               else
+               {
+                       /* LACON arc --- assume it's satisfied and recurse... */
+                       /* ... but first, assert it doesn't lead directly to post state */
+                       Assert(ca->to != cnfa->post);
+
+                       traverse_lacons(cnfa, ca->to, arcs_count, arcs, arcs_len);
+               }
+       }
+}
+
+/*
+ * Get number of outgoing NFA arcs of state number "st".
  */
 int
 pg_reg_getnumoutarcs(const regex_t *regex, int st)
 {
        struct cnfa *cnfa;
-       struct carc *ca;
-       int                     count;
+       int                     arcs_count;
 
        assert(regex != NULL && regex->re_magic == REMAGIC);
        cnfa = &((struct guts *) regex->re_guts)->search;
 
        if (st < 0 || st >= cnfa->nstates)
                return 0;
-       count = 0;
-       for (ca = cnfa->states[st]; ca->co != COLORLESS; ca++)
-       {
-               if (ca->co < cnfa->ncolors)
-                       count++;
-       }
-       return count;
+       arcs_count = 0;
+       traverse_lacons(cnfa, st, &arcs_count, NULL, 0);
+       return arcs_count;
 }
 
 /*
@@ -107,24 +156,15 @@ pg_reg_getoutarcs(const regex_t *regex, int st,
                                  regex_arc_t *arcs, int arcs_len)
 {
        struct cnfa *cnfa;
-       struct carc *ca;
+       int                     arcs_count;
 
        assert(regex != NULL && regex->re_magic == REMAGIC);
        cnfa = &((struct guts *) regex->re_guts)->search;
 
        if (st < 0 || st >= cnfa->nstates || arcs_len <= 0)
                return;
-       for (ca = cnfa->states[st]; ca->co != COLORLESS; ca++)
-       {
-               if (ca->co < cnfa->ncolors)
-               {
-                       arcs->co = ca->co;
-                       arcs->to = ca->to;
-                       arcs++;
-                       if (--arcs_len == 0)
-                               break;
-               }
-       }
+       arcs_count = 0;
+       traverse_lacons(cnfa, st, &arcs_count, arcs, arcs_len);
 }
 
 /*