Fix lquery's behavior for consecutive '*' items.

Something like "*{2}.*{3}" should presumably mean the same as
"*{5}", but it didn't.  Improve that.

Get rid of an undocumented and remarkably ugly (though not, as far as
I can tell, actually unsafe) static variable in favor of passing more
arguments to checkCond().

Reverse-engineer some commentary.  This function, like all of ltree,
is still far short of what I would consider the minimum acceptable
level of internal documentation, but at least now it has more than
zero comments.

Although this certainly seems like a bug fix, people might not
thank us for changing query behavior in stable branches, so
no back-patch.

Nikita Glukhov, with cosmetic improvements by me

Discussion: https://postgr.es/m/CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com

diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out
index 41e7f947875b7e7ca990c799b9e2639956a00409..1b31dbcec2f27d4ecb306e32650b1b1f7d42b2dc 100644 (file)
--- a/contrib/ltree/expected/ltree.out
+++ b/contrib/ltree/expected/ltree.out
@@ -959,6 +959,30 @@ SELECT 'a.b.c.d.e'::ltree ~ '*.!b.*.!c.*';
  f
 (1 row)
 
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{2}.*{2}';
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{2}.e';
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{4}';
+ ?column? 
+----------
+ f
+(1 row)
+
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{5}.*';
+ ?column? 
+----------
+ f
+(1 row)
+
 SELECT 'QWER_TY'::ltree ~ 'q%@*';
  ?column? 
 ----------

diff --git a/contrib/ltree/lquery_op.c b/contrib/ltree/lquery_op.c
index bfbcee6db49773640d8786adb498a963ae985150..5c7afe5d54167133eeeaa054b6c4ec1b9c2c1907 100644 (file)
--- a/contrib/ltree/lquery_op.c
+++ b/contrib/ltree/lquery_op.c
@@ -98,6 +98,11 @@ ltree_strncasecmp(const char *a, const char *b, size_t s)
    return res;
 }
 
+/*
+ * See if a (non-star) lquery_level matches an ltree_level
+ *
+ * Does not consider level's possible LQL_NOT flag.
+ */
 static bool
 checkLevel(lquery_level *curq, ltree_level *curt)
 {
@@ -136,42 +141,49 @@ printFieldNot(FieldNot *fn ) {
 }
 */
 
-static struct
-{
-   bool        muse;
-   uint32      high_pos;
-}          SomeStack =
-
-{
-   false, 0,
-};
-
+/*
+ * Try to match an lquery (of query_numlevel items) to an ltree (of
+ * tree_numlevel items)
+ *
+ * If the query contains any NOT flags, "ptr" must point to a FieldNot
+ * workspace initialized with ptr->q == NULL.  Otherwise it can be NULL.
+ * (LQL_NOT flags will be ignored if ptr == NULL.)
+ *
+ * high_pos is the last ltree position the first lquery item is allowed
+ * to match at; it should be zero for external calls.
+ *
+ * force_advance must be false except in internal recursive calls.
+ */
 static bool
-checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_numlevel, FieldNot *ptr)
+checkCond(lquery_level *curq, int query_numlevel,
+         ltree_level *curt, int tree_numlevel,
+         FieldNot *ptr,
+         uint32 high_pos,
+         bool force_advance)
 {
-   uint32      low_pos = 0,
-               high_pos = 0,
-               cur_tpos = 0;
-   int         tlen = tree_numlevel,
+   uint32      low_pos = 0,    /* first allowed ltree position for match */
+               cur_tpos = 0;   /* ltree position of curt */
+   int         tlen = tree_numlevel,   /* counts of remaining items */
                qlen = query_numlevel;
-   int         isok;
    lquery_level *prevq = NULL;
-   ltree_level *prevt = NULL;
 
-   if (SomeStack.muse)
+   /* advance curq (setting up prevq) if requested */
+   if (force_advance)
    {
-       high_pos = SomeStack.high_pos;
-       qlen--;
+       Assert(qlen > 0);
        prevq = curq;
        curq = LQL_NEXT(curq);
-       SomeStack.muse = false;
+       qlen--;
    }
 
    while (tlen > 0 && qlen > 0)
    {
        if (curq->numvar)
        {
-           prevt = curt;
+           /* Current query item is not '*' */
+           ltree_level *prevt = curt;
+
+           /* skip tree items that must be ignored due to prior * items */
            while (cur_tpos < low_pos)
            {
                curt = LEVEL_NEXT(curt);
@@ -183,8 +195,9 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
                    ptr->nt++;
            }
 
-           if (ptr && curq->flag & LQL_NOT)
+           if (ptr && (curq->flag & LQL_NOT))
            {
+               /* Deal with a NOT item */
                if (!(prevq && prevq->numvar == 0))
                    prevq = curq;
                if (ptr->q == NULL)
@@ -212,25 +225,30 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
            }
            else
            {
-               isok = false;
+               /* Not a NOT item, check for normal match */
+               bool        isok = false;
+
                while (cur_tpos <= high_pos && tlen > 0 && !isok)
                {
                    isok = checkLevel(curq, curt);
                    curt = LEVEL_NEXT(curt);
                    tlen--;
                    cur_tpos++;
-                   if (isok && prevq && prevq->numvar == 0 && tlen > 0 && cur_tpos <= high_pos)
+                   if (isok && prevq && prevq->numvar == 0 &&
+                       tlen > 0 && cur_tpos <= high_pos)
                    {
                        FieldNot    tmpptr;
 
                        if (ptr)
                            memcpy(&tmpptr, ptr, sizeof(FieldNot));
-                       SomeStack.high_pos = high_pos - cur_tpos;
-                       SomeStack.muse = true;
-                       if (checkCond(prevq, qlen + 1, curt, tlen, (ptr) ? &tmpptr : NULL))
+                       if (checkCond(prevq, qlen + 1,
+                                     curt, tlen,
+                                     (ptr) ? &tmpptr : NULL,
+                                     high_pos - cur_tpos,
+                                     true))
                            return true;
                    }
-                   if (!isok && ptr)
+                   if (!isok && ptr && ptr->q)
                        ptr->nt++;
                }
                if (!isok)
@@ -238,7 +256,11 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
 
                if (ptr && ptr->q)
                {
-                   if (checkCond(ptr->q, ptr->nq, ptr->t, ptr->nt, NULL))
+                   if (checkCond(ptr->q, ptr->nq,
+                                 ptr->t, ptr->nt,
+                                 NULL,
+                                 0,
+                                 false))
                        return false;
                    ptr->q = NULL;
                }
@@ -248,8 +270,14 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
        }
        else
        {
-           low_pos = cur_tpos + curq->low;
-           high_pos = cur_tpos + curq->high;
+           /* Current query item is '*' */
+           low_pos += curq->low;
+
+           if (low_pos > tree_numlevel)
+               return false;
+
+           high_pos = Min(high_pos + curq->high, tree_numlevel);
+
            if (ptr && ptr->q)
            {
                ptr->nq++;
@@ -263,9 +291,11 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
        qlen--;
    }
 
+   /* Fail if we've already run out of ltree items */
    if (low_pos > tree_numlevel || tree_numlevel > high_pos)
        return false;
 
+   /* Remaining lquery items must be NOT or '*' items */
    while (qlen > 0)
    {
        if (curq->numvar)
@@ -275,18 +305,29 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
        }
        else
        {
-           low_pos = cur_tpos + curq->low;
-           high_pos = cur_tpos + curq->high;
+           low_pos += curq->low;
+
+           if (low_pos > tree_numlevel)
+               return false;
+
+           high_pos = Min(high_pos + curq->high, tree_numlevel);
        }
 
        curq = LQL_NEXT(curq);
        qlen--;
    }
 
+   /* Fail if trailing '*'s require more ltree items than we have */
    if (low_pos > tree_numlevel || tree_numlevel > high_pos)
        return false;
 
-   if (ptr && ptr->q && checkCond(ptr->q, ptr->nq, ptr->t, ptr->nt, NULL))
+   /* Finish pending NOT check, if any */
+   if (ptr && ptr->q &&
+       checkCond(ptr->q, ptr->nq,
+                 ptr->t, ptr->nt,
+                 NULL,
+                 0,
+                 false))
        return false;
 
    return true;
@@ -306,12 +347,18 @@ ltq_regex(PG_FUNCTION_ARGS)
        fn.q = NULL;
 
        res = checkCond(LQUERY_FIRST(query), query->numlevel,
-                       LTREE_FIRST(tree), tree->numlevel, &fn);
+                       LTREE_FIRST(tree), tree->numlevel,
+                       &fn,
+                       0,
+                       false);
    }
    else
    {
        res = checkCond(LQUERY_FIRST(query), query->numlevel,
-                       LTREE_FIRST(tree), tree->numlevel, NULL);
+                       LTREE_FIRST(tree), tree->numlevel,
+                       NULL,
+                       0,
+                       false);
    }
 
    PG_FREE_IF_COPY(tree, 0);

diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index a5608ca1f6dee7c49715197d68fd4daffd4f14f2..2c483045dbbb8f52cba2fc2c65f3da50d1deb41d 100644 (file)
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -34,7 +34,7 @@ typedef struct
 {
    int32       val;
    uint16      len;
-   uint8       flag;
+   uint8       flag;           /* see LVAR_xxx flags below */
    char        name[FLEXIBLE_ARRAY_MEMBER];
 } lquery_variant;
 
@@ -47,11 +47,12 @@ typedef struct
 
 typedef struct
 {
-   uint16      totallen;
-   uint16      flag;
-   uint16      numvar;
-   uint16      low;
-   uint16      high;
+   uint16      totallen;       /* total length of this level, in bytes */
+   uint16      flag;           /* see LQL_xxx flags below */
+   uint16      numvar;         /* number of variants; 0 means '*' */
+   uint16      low;            /* minimum repeat count for '*' */
+   uint16      high;           /* maximum repeat count for '*' */
+   /* Array of maxalign'd lquery_variant structs follows: */
    char        variants[FLEXIBLE_ARRAY_MEMBER];
 } lquery_level;
 
@@ -60,6 +61,7 @@ typedef struct
 #define LQL_FIRST(x)   ( (lquery_variant*)( ((char*)(x))+LQL_HDRSIZE ) )
 
 #define LQL_NOT        0x10
+
 #ifdef LOWER_NODE
 #define FLG_CANLOOKSIGN(x) ( ( (x) & ( LQL_NOT | LVAR_ANYEND | LVAR_SUBLEXEME ) ) == 0 )
 #else
@@ -70,9 +72,10 @@ typedef struct
 typedef struct
 {
    int32       vl_len_;        /* varlena header (do not touch directly!) */
-   uint16      numlevel;
+   uint16      numlevel;       /* number of lquery_levels */
    uint16      firstgood;
-   uint16      flag;
+   uint16      flag;           /* see LQUERY_xxx flags below */
+   /* Array of maxalign'd lquery_level structs follows: */
    char        data[FLEXIBLE_ARRAY_MEMBER];
 } lquery;
 

diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql
index fca3ae645d24a1b95fd0ab96a202a09be79bacf1..1de0b2af12f4f3deed4e455b6527c036ac813dd2 100644 (file)
--- a/contrib/ltree/sql/ltree.sql
+++ b/contrib/ltree/sql/ltree.sql
@@ -179,7 +179,10 @@ SELECT 'a.b.c.d.e'::ltree ~ 'a.!b.*{1}.!c.*';
 SELECT 'a.b.c.d.e'::ltree ~ '!b.*{1}.!c.*';
 SELECT 'a.b.c.d.e'::ltree ~ '*.!b.*{1}.!c.*';
 SELECT 'a.b.c.d.e'::ltree ~ '*.!b.*.!c.*';
-
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{2}.*{2}';
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{2}.e';
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{4}';
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{5}.*';
 
 SELECT 'QWER_TY'::ltree ~ 'q%@*';
 SELECT 'QWER_TY'::ltree ~ 'Q_t%@*';