]> granicus.if.org Git - postgresql/commitdiff
Fix bogus handling of XQuery regex option flags.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Sep 2019 19:39:51 +0000 (15:39 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Sep 2019 19:39:51 +0000 (15:39 -0400)
The SQL spec defers to XQuery to define what the option flags are
for LIKE_REGEX patterns.  XQuery says that:
* 's' allows the dot character to match newlines, which by
  default it will not;
* 'm' allows ^ and $ to match at newlines, not only at the
  start/end of the whole string.
Thus, these are *not* inverses as they are for the similarly-named
POSIX options, and neither one corresponds to the POSIX 'n' option.
Fortunately, Spencer's library does expose these two behaviors as
separately twiddlable flags, so we just have to fix the mapping from
JSP flag bits to REG flag bits.  I also chose to rename the symbol
for 's' to DOTALL, to make it clearer that it's not the inverse
of MLINE.

Also, XQuery says that if the 'q' flag "is used together with the m, s,
or x flag, that flag has no effect".  I read this as saying that 'q'
overrides the other flags; whoever wrote our code seems to have read
it backwards.

Lastly, while XQuery's 'x' flag is related to what Spencer's code
does for REG_EXPANDED, it's not the same or a subset.  It seems best
to treat XQuery's 'x' as unimplemented for now.  Maybe later we can
expand our regex code to offer 'x'-style parsing as a separate option.

While at it, refactor the jsonpath code so that (a) there's only
one copy of the flag transformation logic not two, and (b) the
processing of flags is independent of the order in which the flags
are written.

We need some documentation updates to go with this, but I'll
tackle that separately.

Back-patch to v12 where this code originated.

Discussion: https://postgr.es/m/CAPpHfdvDci4iqNF9fhRkTqhe-5_8HmzeLt56drH%2B_Rv2rNRqfg@mail.gmail.com
Reference: https://www.w3.org/TR/2017/REC-xpath-functions-31-20170321/#flags

src/backend/utils/adt/jsonpath.c
src/backend/utils/adt/jsonpath_exec.c
src/backend/utils/adt/jsonpath_gram.y
src/include/utils/jsonpath.h
src/test/regress/expected/jsonb_jsonpath.out
src/test/regress/expected/jsonpath.out
src/test/regress/sql/jsonb_jsonpath.sql

index 87ae60e490f6232bf211276e50f7fb9ed6ee8784..7f322485e7b1f399bb6d4ecae5ab403b52bb65bd 100644 (file)
@@ -557,7 +557,7 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
 
                                if (v->content.like_regex.flags & JSP_REGEX_ICASE)
                                        appendStringInfoChar(buf, 'i');
-                               if (v->content.like_regex.flags & JSP_REGEX_SLINE)
+                               if (v->content.like_regex.flags & JSP_REGEX_DOTALL)
                                        appendStringInfoChar(buf, 's');
                                if (v->content.like_regex.flags & JSP_REGEX_MLINE)
                                        appendStringInfoChar(buf, 'm');
index 21106e1da866f7af8d4ca5ae4f85e9863643d6bd..565b00c4266006c8ea9e5fdefc8c88231cfd8350 100644 (file)
@@ -1646,34 +1646,10 @@ executeLikeRegex(JsonPathItem *jsp, JsonbValue *str, JsonbValue *rarg,
        /* Cache regex text and converted flags. */
        if (!cxt->regex)
        {
-               uint32          flags = jsp->content.like_regex.flags;
-
                cxt->regex =
                        cstring_to_text_with_len(jsp->content.like_regex.pattern,
                                                                         jsp->content.like_regex.patternlen);
-
-               /* Convert regex flags. */
-               cxt->cflags = REG_ADVANCED;
-
-               if (flags & JSP_REGEX_ICASE)
-                       cxt->cflags |= REG_ICASE;
-               if (flags & JSP_REGEX_MLINE)
-                       cxt->cflags |= REG_NEWLINE;
-               if (flags & JSP_REGEX_SLINE)
-                       cxt->cflags &= ~REG_NEWLINE;
-               if (flags & JSP_REGEX_WSPACE)
-                       cxt->cflags |= REG_EXPANDED;
-
-               /*
-                * 'q' flag can work together only with 'i'.  When other is specified,
-                * then 'q' has no effect.
-                */
-               if ((flags & JSP_REGEX_QUOTE) &&
-                       !(flags & (JSP_REGEX_MLINE | JSP_REGEX_SLINE | JSP_REGEX_WSPACE)))
-               {
-                       cxt->cflags &= ~REG_ADVANCED;
-                       cxt->cflags |= REG_QUOTE;
-               }
+               cxt->cflags = jspConvertRegexFlags(jsp->content.like_regex.flags);
        }
 
        if (RE_compile_and_execute(cxt->regex, str->val.string.val,
index 91b4b2f5985ca7ce6c0fcfb8394ee48ced2c48c7..1725502ede213aeee9922a1c3575cae00bb52f90 100644 (file)
@@ -481,42 +481,32 @@ makeItemLikeRegex(JsonPathParseItem *expr, JsonPathString *pattern,
 {
        JsonPathParseItem  *v = makeItemType(jpiLikeRegex);
        int                                     i;
-       int                                     cflags = REG_ADVANCED;
+       int                                     cflags;
 
        v->value.like_regex.expr = expr;
        v->value.like_regex.pattern = pattern->val;
        v->value.like_regex.patternlen = pattern->len;
-       v->value.like_regex.flags = 0;
 
+       /* Parse the flags string, convert to bitmask.  Duplicate flags are OK. */
+       v->value.like_regex.flags = 0;
        for (i = 0; flags && i < flags->len; i++)
        {
                switch (flags->val[i])
                {
                        case 'i':
                                v->value.like_regex.flags |= JSP_REGEX_ICASE;
-                               cflags |= REG_ICASE;
                                break;
                        case 's':
-                               v->value.like_regex.flags &= ~JSP_REGEX_MLINE;
-                               v->value.like_regex.flags |= JSP_REGEX_SLINE;
-                               cflags |= REG_NEWLINE;
+                               v->value.like_regex.flags |= JSP_REGEX_DOTALL;
                                break;
                        case 'm':
-                               v->value.like_regex.flags &= ~JSP_REGEX_SLINE;
                                v->value.like_regex.flags |= JSP_REGEX_MLINE;
-                               cflags &= ~REG_NEWLINE;
                                break;
                        case 'x':
                                v->value.like_regex.flags |= JSP_REGEX_WSPACE;
-                               cflags |= REG_EXPANDED;
                                break;
                        case 'q':
                                v->value.like_regex.flags |= JSP_REGEX_QUOTE;
-                               if (!(v->value.like_regex.flags & (JSP_REGEX_MLINE | JSP_REGEX_SLINE | JSP_REGEX_WSPACE)))
-                               {
-                                       cflags &= ~REG_ADVANCED;
-                                       cflags |= REG_QUOTE;
-                               }
                                break;
                        default:
                                ereport(ERROR,
@@ -528,6 +518,9 @@ makeItemLikeRegex(JsonPathParseItem *expr, JsonPathString *pattern,
                }
        }
 
+       /* Convert flags to what RE_compile_and_cache needs */
+       cflags = jspConvertRegexFlags(v->value.like_regex.flags);
+
        /* check regex validity */
        (void) RE_compile_and_cache(cstring_to_text_with_len(pattern->val,
                                                                                                                 pattern->len),
@@ -536,6 +529,49 @@ makeItemLikeRegex(JsonPathParseItem *expr, JsonPathString *pattern,
        return v;
 }
 
+/*
+ * Convert from XQuery regex flags to those recognized by our regex library.
+ */
+int
+jspConvertRegexFlags(uint32 xflags)
+{
+       /* By default, XQuery is very nearly the same as Spencer's AREs */
+       int                     cflags = REG_ADVANCED;
+
+       /* Ignore-case means the same thing, too, modulo locale issues */
+       if (xflags & JSP_REGEX_ICASE)
+               cflags |= REG_ICASE;
+
+       /* Per XQuery spec, if 'q' is specified then 'm', 's', 'x' are ignored */
+       if (xflags & JSP_REGEX_QUOTE)
+       {
+               cflags &= ~REG_ADVANCED;
+               cflags |= REG_QUOTE;
+       }
+       else
+       {
+               /* Note that dotall mode is the default in POSIX */
+               if (!(xflags & JSP_REGEX_DOTALL))
+                       cflags |= REG_NLSTOP;
+               if (xflags & JSP_REGEX_MLINE)
+                       cflags |= REG_NLANCH;
+
+               /*
+                * XQuery's 'x' mode is related to Spencer's expanded mode, but it's
+                * not really enough alike to justify treating JSP_REGEX_WSPACE as
+                * REG_EXPANDED.  For now we treat 'x' as unimplemented; perhaps in
+                * future we'll modify the regex library to have an option for
+                * XQuery-style ignore-whitespace mode.
+                */
+               if (xflags & JSP_REGEX_WSPACE)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("XQuery \"x\" flag (expanded regular expressions) is not implemented")));
+       }
+
+       return cflags;
+}
+
 /*
  * jsonpath_scan.l is compiled as part of jsonpath_gram.y.  Currently, this is
  * unavoidable because jsonpath_gram does not create a .h file to export its
index 40ad5fda928c1ebba0c4dc0fe0bd8d5c6477422a..8458346bd4ccf44e6ec7206f61c3b8b71940adfa 100644 (file)
@@ -88,9 +88,9 @@ typedef enum JsonPathItemType
 
 /* XQuery regex mode flags for LIKE_REGEX predicate */
 #define JSP_REGEX_ICASE                0x01    /* i flag, case insensitive */
-#define JSP_REGEX_SLINE                0x02    /* s flag, single-line mode */
-#define JSP_REGEX_MLINE                0x04    /* m flag, multi-line mode */
-#define JSP_REGEX_WSPACE       0x08    /* x flag, expanded syntax */
+#define JSP_REGEX_DOTALL       0x02    /* s flag, dot matches newline */
+#define JSP_REGEX_MLINE                0x04    /* m flag, ^/$ match at newlines */
+#define JSP_REGEX_WSPACE       0x08    /* x flag, ignore whitespace in pattern */
 #define JSP_REGEX_QUOTE                0x10    /* q flag, no special characters */
 
 /*
@@ -245,4 +245,6 @@ typedef struct JsonPathParseResult
 
 extern JsonPathParseResult *parsejsonpath(const char *str, int len);
 
+extern int     jspConvertRegexFlags(uint32 xflags);
+
 #endif
index 0202667a1f732d04a3285f82594c76e08789b428..d9618f2d8879902b1e588860f934d0b5a786ba6e 100644 (file)
@@ -1592,14 +1592,14 @@ select jsonb_path_query('[null, 1, "abd", "abdabc"]', 'lax $[*] ? ((@ starts wit
  1
 (2 rows)
 
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^ab.*c")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c")');
  jsonb_path_query 
 ------------------
  "abc"
  "abdacb"
 (2 rows)
 
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^a  b.*  c " flag "ix")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "i")');
  jsonb_path_query 
 ------------------
  "abc"
@@ -1607,7 +1607,7 @@ select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "
  "abdacb"
 (3 rows)
 
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "m")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "m")');
  jsonb_path_query 
 ------------------
  "abc"
@@ -1615,12 +1615,13 @@ select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "
  "adc\nabc"
 (3 rows)
 
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "s")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "s")');
  jsonb_path_query 
 ------------------
  "abc"
  "abdacb"
-(2 rows)
+ "ab\nadc"
+(3 rows)
 
 select jsonb_path_query('[null, 1, "a\b", "a\\b", "^a\\b$"]', 'lax $[*] ? (@ like_regex "a\\b" flag "q")');
  jsonb_path_query 
index ecdd453942b2ca48a6082152652c3cb49cc2fbef..ea42ae367a3a6073bd875e444b04ecd4ba6aebc0 100644 (file)
@@ -442,17 +442,15 @@ select '$ ? (@ like_regex "pattern" flag "is")'::jsonpath;
 (1 row)
 
 select '$ ? (@ like_regex "pattern" flag "isim")'::jsonpath;
-               jsonpath               
---------------------------------------
- $?(@ like_regex "pattern" flag "im")
+               jsonpath                
+---------------------------------------
+ $?(@ like_regex "pattern" flag "ism")
 (1 row)
 
 select '$ ? (@ like_regex "pattern" flag "xsms")'::jsonpath;
-               jsonpath               
---------------------------------------
- $?(@ like_regex "pattern" flag "sx")
-(1 row)
-
+ERROR:  XQuery "x" flag (expanded regular expressions) is not implemented
+LINE 1: select '$ ? (@ like_regex "pattern" flag "xsms")'::jsonpath;
+               ^
 select '$ ? (@ like_regex "pattern" flag "q")'::jsonpath;
               jsonpath               
 -------------------------------------
@@ -466,9 +464,9 @@ select '$ ? (@ like_regex "pattern" flag "iq")'::jsonpath;
 (1 row)
 
 select '$ ? (@ like_regex "pattern" flag "smixq")'::jsonpath;
-                jsonpath                
-----------------------------------------
- $?(@ like_regex "pattern" flag "imxq")
+                jsonpath                 
+-----------------------------------------
+ $?(@ like_regex "pattern" flag "ismxq")
 (1 row)
 
 select '$ ? (@ like_regex "pattern" flag "a")'::jsonpath;
index e7629fb7f9d99632f8147559d1ed9a68ed1ce7ff..ae8549d55364d6480d060b46a8410f319811e1cf 100644 (file)
@@ -335,10 +335,10 @@ select jsonb_path_query('[[null, 1, "abc", "abcabc"]]', 'lax $ ? (@[*] starts wi
 select jsonb_path_query('[[null, 1, "abd", "abdabc"]]', 'lax $ ? ((@[*] starts with "abc") is unknown)');
 select jsonb_path_query('[null, 1, "abd", "abdabc"]', 'lax $[*] ? ((@ starts with "abc") is unknown)');
 
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^ab.*c")');
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^a  b.*  c " flag "ix")');
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "m")');
-select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "adc\nabc", "babc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "s")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "i")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "m")');
+select jsonb_path_query('[null, 1, "abc", "abd", "aBdC", "abdacb", "babc", "adc\nabc", "ab\nadc"]', 'lax $[*] ? (@ like_regex "^ab.*c" flag "s")');
 select jsonb_path_query('[null, 1, "a\b", "a\\b", "^a\\b$"]', 'lax $[*] ? (@ like_regex "a\\b" flag "q")');
 select jsonb_path_query('[null, 1, "a\b", "a\\b", "^a\\b$"]', 'lax $[*] ? (@ like_regex "a\\b" flag "")');
 select jsonb_path_query('[null, 1, "a\b", "a\\b", "^a\\b$"]', 'lax $[*] ? (@ like_regex "^a\\b$" flag "q")');