Fix bogus tree-flattening logic in QTNTernary().

QTNTernary() contains logic to flatten, eg, '(a & b) & c' into 'a & b & c',
which is all well and good, but it tries to do that to NOT nodes as well,
so that '!!a' gets changed to '!a'.  Explicitly restrict the conversion to
be done only on AND and OR nodes, and add a test case illustrating the bug.

In passing, provide some comments for the sadly naked functions in
tsquery_util.c, and simplify some baroque logic in QTNFree(), which
I think may have been leaking some items it intended to free.

Noted while investigating a complaint from Andreas Seltenreich.
Back-patch to all supported versions.

diff --git a/src/backend/utils/adt/tsquery_util.c b/src/backend/utils/adt/tsquery_util.c
index 9003702599e5d9ce7fd6d87bbb9752b78a641108..e1851baec7c3f5cebd4892ca2a1ea770abe964ef 100644 (file)
--- a/src/backend/utils/adt/tsquery_util.c
+++ b/src/backend/utils/adt/tsquery_util.c
@@ -17,6 +17,9 @@
 #include "tsearch/ts_utils.h"
 #include "miscadmin.h"
 
+/*
+ * Build QTNode tree for a tsquery given in QueryItem array format.
+ */
 QTNode *
 QT2QTN(QueryItem *in, char *operand)
 {
@@ -50,6 +53,12 @@ QT2QTN(QueryItem *in, char *operand)
        return node;
 }
 
+/*
+ * Free a QTNode tree.
+ *
+ * Referenced "word" and "valnode" items are freed if marked as transient
+ * by flags.
+ */
 void
 QTNFree(QTNode *in)
 {
@@ -62,26 +71,27 @@ QTNFree(QTNode *in)
        if (in->valnode->type == QI_VAL && in->word && (in->flags & QTN_WORDFREE) != 0)
                pfree(in->word);
 
-       if (in->child)
+       if (in->valnode->type == QI_OPR)
        {
-               if (in->valnode)
-               {
-                       if (in->valnode->type == QI_OPR && in->nchild > 0)
-                       {
-                               int                     i;
-
-                               for (i = 0; i < in->nchild; i++)
-                                       QTNFree(in->child[i]);
-                       }
-                       if (in->flags & QTN_NEEDFREE)
-                               pfree(in->valnode);
-               }
-               pfree(in->child);
+               int                     i;
+
+               for (i = 0; i < in->nchild; i++)
+                       QTNFree(in->child[i]);
        }
+       if (in->child)
+               pfree(in->child);
+
+       if (in->flags & QTN_NEEDFREE)
+               pfree(in->valnode);
 
        pfree(in);
 }
 
+/*
+ * Sort comparator for QTNodes.
+ *
+ * The sort order is somewhat arbitrary.
+ */
 int
 QTNodeCompare(QTNode *an, QTNode *bn)
 {
@@ -131,12 +141,19 @@ QTNodeCompare(QTNode *an, QTNode *bn)
        }
 }
 
+/*
+ * qsort comparator for QTNode pointers.
+ */
 static int
 cmpQTN(const void *a, const void *b)
 {
        return QTNodeCompare(*(QTNode *const *) a, *(QTNode *const *) b);
 }
 
+/*
+ * Canonicalize a QTNode tree by sorting the children of AND/OR nodes
+ * into an arbitrary but well-defined order.
+ */
 void
 QTNSort(QTNode *in)
 {
@@ -154,13 +171,16 @@ QTNSort(QTNode *in)
                qsort((void *) in->child, in->nchild, sizeof(QTNode *), cmpQTN);
 }
 
+/*
+ * Are two QTNode trees equal according to QTNodeCompare?
+ */
 bool
 QTNEq(QTNode *a, QTNode *b)
 {
        uint32          sign = a->sign & b->sign;
 
        if (!(sign == a->sign && sign == b->sign))
-               return 0;
+               return false;
 
        return (QTNodeCompare(a, b) == 0) ? true : false;
 }
@@ -186,11 +206,17 @@ QTNTernary(QTNode *in)
        for (i = 0; i < in->nchild; i++)
                QTNTernary(in->child[i]);
 
+       /* Only AND and OR are associative, so don't flatten other node types */
+       if (in->valnode->qoperator.oper != OP_AND &&
+               in->valnode->qoperator.oper != OP_OR)
+               return;
+
        for (i = 0; i < in->nchild; i++)
        {
                QTNode     *cc = in->child[i];
 
-               if (cc->valnode->type == QI_OPR && in->valnode->qoperator.oper == cc->valnode->qoperator.oper)
+               if (cc->valnode->type == QI_OPR &&
+                       in->valnode->qoperator.oper == cc->valnode->qoperator.oper)
                {
                        int                     oldnchild = in->nchild;
 
@@ -229,9 +255,6 @@ QTNBinary(QTNode *in)
        for (i = 0; i < in->nchild; i++)
                QTNBinary(in->child[i]);
 
-       if (in->nchild <= 2)
-               return;
-
        while (in->nchild > 2)
        {
                QTNode     *nn = (QTNode *) palloc0(sizeof(QTNode));
@@ -256,8 +279,9 @@ QTNBinary(QTNode *in)
 }
 
 /*
- * Count the total length of operand string in tree, including '\0'-
- * terminators.
+ * Count the total length of operand strings in tree (including '\0'-
+ * terminators) and the total number of nodes.
+ * Caller must initialize *sumlen and *nnode to zeroes.
  */
 static void
 cntsize(QTNode *in, int *sumlen, int *nnode)
@@ -286,6 +310,10 @@ typedef struct
        char       *curoperand;
 } QTN2QTState;
 
+/*
+ * Recursively convert a QTNode tree into flat tsquery format.
+ * Caller must have allocated arrays of the correct size.
+ */
 static void
 fillQT(QTN2QTState *state, QTNode *in)
 {
@@ -323,6 +351,9 @@ fillQT(QTN2QTState *state, QTNode *in)
        }
 }
 
+/*
+ * Build flat tsquery from a QTNode tree.
+ */
 TSQuery
 QTN2QT(QTNode *in)
 {
@@ -351,6 +382,11 @@ QTN2QT(QTNode *in)
        return out;
 }
 
+/*
+ * Copy a QTNode tree.
+ *
+ * Modifiable copies of the words and valnodes are made, too.
+ */
 QTNode *
 QTNCopy(QTNode *in)
 {
@@ -386,6 +422,9 @@ QTNCopy(QTNode *in)
        return out;
 }
 
+/*
+ * Clear the specified flag bit(s) in all nodes of a QTNode tree.
+ */
 void
 QTNClearFlags(QTNode *in, uint32 flags)
 {

diff --git a/src/test/regress/expected/tsearch.out b/src/test/regress/expected/tsearch.out
index 9341dbe0d77e1723f0c98f7837b5b69b7c748fc3..1466582c3eac5e2b6241b1f34893094292de3dc5 100644 (file)
--- a/src/test/regress/expected/tsearch.out
+++ b/src/test/regress/expected/tsearch.out
@@ -851,6 +851,13 @@ SELECT ts_rewrite('foo & bar & qq & new & york',  'new & york'::tsquery, 'big &
  'foo' & 'bar' & 'qq' & ( 'city' & 'new' & 'york' | ( 'nyc' | 'big' & 'apple' ) )
 (1 row)
 
+SELECT ts_rewrite(ts_rewrite('new & !york ', 'york', '!jersey'),
+                  'jersey', 'mexico');
+       ts_rewrite       
+------------------------
+ 'new' & !( !'mexico' )
+(1 row)
+
 SELECT ts_rewrite('moscow', 'SELECT keyword, sample FROM test_tsquery'::text );
      ts_rewrite      
 ---------------------

diff --git a/src/test/regress/sql/tsearch.sql b/src/test/regress/sql/tsearch.sql
index 9fd12076acedb0b7c2a617466070b49e76f40778..6035d5e4272c9de170cbb66d15ded92db0c97f70 100644 (file)
--- a/src/test/regress/sql/tsearch.sql
+++ b/src/test/regress/sql/tsearch.sql
@@ -305,6 +305,8 @@ SELECT COUNT(*) FROM test_tsquery WHERE keyword >  'new & york';
 RESET enable_seqscan;
 
 SELECT ts_rewrite('foo & bar & qq & new & york',  'new & york'::tsquery, 'big & apple | nyc | new & york & city');
+SELECT ts_rewrite(ts_rewrite('new & !york ', 'york', '!jersey'),
+                  'jersey', 'mexico');
 
 SELECT ts_rewrite('moscow', 'SELECT keyword, sample FROM test_tsquery'::text );
 SELECT ts_rewrite('moscow & hotel', 'SELECT keyword, sample FROM test_tsquery'::text );