Re: [HACKERS] [sqlsmith] Crash in tsquery_rewrite/QTNBinary

[Tom Lane]

Artur Zakirov  writes:
> 2016-12-07 9:06 GMT+03:00 Andreas Seltenreich :
>> the following query crashes master as of 4212cb7.

> It happens because 'I' is stop word and substitute query becomes
> empty. And for queries above we need recursive dropvoidsubtree()
> function. Without this patch this function cleans only first level of
> tree. And query above becomes: '6 | void'.

> Firstly I made recursive dropvoidsubtree(). But attached patch cleans
> query tree in dofindsubquery() to avoid extra tree scan.

This patch looks good to me.  I have to admit that I'd been suspicious
of dropvoidsubtree() the last time I looked at this code, but I didn't
have adequate reason to touch it.  Pushed with some minor comment
adjustments.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [sqlsmith] Crash in tsquery_rewrite/QTNBinary

[Artur Zakirov]

Hi,

2016-12-07 9:06 GMT+03:00 Andreas Seltenreich :
> Hi,
>
> the following query crashes master as of 4212cb7.
>
> select ts_rewrite(
>   tsquery_phrase(
>  tsquery $$'sanct' & 'peter'$$,
>  tsquery $$'5' <-> '6'$$,
>  42),
>   tsquery $$'5' <-> '6'$$,
>   plainto_tsquery('I') );
>

This happens also for queries:

select ts_rewrite(
  to_tsquery('5 & (6 | 5)'),
  to_tsquery('5'),
  to_tsquery('I'));

select ts_rewrite(
  to_tsquery('5 & 6 <-> 5'),
  to_tsquery('5'),
  to_tsquery('I'));

It happens because 'I' is stop word and substitute query becomes
empty. And for queries above we need recursive dropvoidsubtree()
function. Without this patch this function cleans only first level of
tree. And query above becomes: '6 | void'.

Firstly I made recursive dropvoidsubtree(). But attached patch cleans
query tree in dofindsubquery() to avoid extra tree scan.

-- 
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git a/src/backend/utils/adt/tsquery_rewrite.c 
b/src/backend/utils/adt/tsquery_rewrite.c
index e5bed6e..00174bd 100644
--- a/src/backend/utils/adt/tsquery_rewrite.c
+++ b/src/backend/utils/adt/tsquery_rewrite.c
@@ -186,6 +186,15 @@ findeq(QTNode *node, QTNode *ex, QTNode *subs, bool 
*isfind)
  * Recursive guts of findsubquery(): attempt to replace "ex" with "subs"
  * at the root node, and if we failed to do so, recursively match against
  * child nodes.
+ *
+ * Delete any void subtrees. In the following example '5' is replaced by empty
+ * operand:
+ *
+ *AND   ->6
+ *   /   \
+ *  5PHRASE
+ *   / \
+ *  6  5
  */
 static QTNode *
 dofindsubquery(QTNode *root, QTNode *ex, QTNode *subs, bool *isfind)
@@ -200,39 +209,20 @@ dofindsubquery(QTNode *root, QTNode *ex, QTNode *subs, 
bool *isfind)
 
if (root && (root->flags & QTN_NOCHANGE) == 0 && root->valnode->type == 
QI_OPR)
{
-   int i;
-
-   for (i = 0; i < root->nchild; i++)
-   root->child[i] = dofindsubquery(root->child[i], ex, 
subs, isfind);
-   }
-
-   return root;
-}
-
-/*
- * Delete any void subtrees that may have been inserted when the replacement
- * subtree is void.
- */
-static QTNode *
-dropvoidsubtree(QTNode *root)
-{
-   if (!root)
-   return NULL;
-
-   if (root->valnode->type == QI_OPR)
-   {
int i,
j = 0;
 
for (i = 0; i < root->nchild; i++)
{
-   if (root->child[i])
-   {
-   root->child[j] = root->child[i];
+   root->child[j] = dofindsubquery(root->child[i], ex, 
subs, isfind);
+   if (root->child[j])
j++;
-   }
}
 
+   /*
+* Delete any void subtrees that may have been inserted when 
the replacement
+* subtree is void.
+*/
root->nchild = j;
 
if (root->nchild == 0)
@@ -267,9 +257,6 @@ findsubquery(QTNode *root, QTNode *ex, QTNode *subs, bool 
*isfind)
 
root = dofindsubquery(root, ex, subs, );
 
-   if (!subs && DidFind)
-   root = dropvoidsubtree(root);
-
if (isfind)
*isfind = DidFind;
 
diff --git a/src/test/regress/expected/tsearch.out 
b/src/test/regress/expected/tsearch.out
index 55d6a85..9c20aaf 100644
--- a/src/test/regress/expected/tsearch.out
+++ b/src/test/regress/expected/tsearch.out
@@ -1251,6 +1251,14 @@ SELECT ts_rewrite('5 <-> (6 | 8)', 'SELECT keyword, 
sample FROM test_tsquery'::t
  '5' <-> '7' | '5' <-> '8'
 (1 row)
 
+-- Check empty substitute
+SELECT ts_rewrite(to_tsquery('5 & 6 <-> 5'), to_tsquery('5'), to_tsquery('I'));
+NOTICE:  text-search query contains only stop words or doesn't contain 
lexemes, ignored
+ ts_rewrite 
+
+ '6'
+(1 row)
+
 SELECT keyword FROM test_tsquery WHERE keyword @> 'new';
 keyword 
 
diff --git a/src/test/regress/sql/tsearch.sql b/src/test/regress/sql/tsearch.sql
index afd990e..8752344 100644
--- a/src/test/regress/sql/tsearch.sql
+++ b/src/test/regress/sql/tsearch.sql
@@ -418,6 +418,8 @@ SELECT ts_rewrite('1 & (2 <2> 3)', 'SELECT keyword, sample 
FROM test_tsquery'::t
 SELECT ts_rewrite('5 <-> (1 & (2 <-> 3))', 'SELECT keyword, sample FROM 
test_tsquery'::text );
 SELECT ts_rewrite('5 <-> (6 | 8)', 'SELECT keyword, sample FROM 
test_tsquery'::text );
 
+-- Check empty substitute
+SELECT ts_rewrite(to_tsquery('5 & 6 <-> 5'), to_tsquery('5'), to_tsquery('I'));
 
 SELECT keyword FROM test_tsquery WHERE keyword @> 'new';
 SELECT keyword FROM test_tsquery WHERE keyword @> 'moscow';

-- 
Sent via pgsql-hackers mailing list