From: Tom Lane Date: Thu, 13 Apr 2017 21:18:35 +0000 (-0400) Subject: Fix regexport.c to behave sanely with lookaround constraints. X-Git-Tag: REL_10_BETA1~270 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6cfaffc0ddc73dab6857c094f98b28761898cc6d;p=postgresql 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 df6c1ff513..6b2fa7eb49 100644 --- 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 806f3eb704..2619ad5ee6 100644 --- 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 3856ff873f..371c1f9da6 100644 --- 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); } /*