Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit2604438

Browse files
committed
Fix handling of phrase operator removal while removing tsquery stopwords.
The distance of a removed phrase operator should propagate up to aparent phrase operator if there is one, but this only worked correctlyin left-deep trees. Throwing in a few parentheses confused it completely,as indeed was illustrated by bizarre results in existing regression testcases.To fix, track unaccounted-for distances that should propagate to the leftand to the right of the current node, rather than trying to make it workwith only one returned distance.Also make some adjustments to behave as well as we can for cases ofintermixed phrase and regular (AND/OR) operators. I don't think it'spossible to be 100% correct for that without a rethinking of the tsqueryrepresentation; for example, maybe we should just not drop stopword nodesat all underneath phrase operators. But this is better than it was,and changing tsquery representation wouldn't be safely back-patchable.While at it, I simplified the API of the clean_fakeval_intree functiona bit by getting rid of the "char *result" output parameter; that wasn'tdoing anything that wasn't redundant with whether the result node isNULL or not, and testing for NULL seems a lot clearer/safer.This is part of a larger project to fix various infelicities in thephrase-search implementation, but this part seems comittable on its own.Back-patch to 9.6 where phrase operators were introduced.Discussion:https://postgr.es/m/28215.1481999808@sss.pgh.pa.usDiscussion:https://postgr.es/m/26706.1482087250@sss.pgh.pa.us
1 parentdd72882 commit2604438

File tree

2 files changed

+119
-82
lines changed

2 files changed

+119
-82
lines changed

‎src/backend/utils/adt/tsquery_cleanup.c

Lines changed: 110 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -207,124 +207,160 @@ clean_NOT(QueryItem *ptr, int *len)
207207
}
208208

209209

210-
#ifdefV_UNKNOWN/* exists in Windows headers */
211-
#undef V_UNKNOWN
212-
#endif
213-
#ifdefV_FALSE/* exists in Solaris headers */
214-
#undef V_FALSE
215-
#endif
216-
217-
/*
218-
* output values for result output parameter of clean_fakeval_intree
219-
*/
220-
#defineV_UNKNOWN0/* the expression can't be evaluated
221-
* statically */
222-
#defineV_TRUE1/* the expression is always true (not
223-
* implemented) */
224-
#defineV_FALSE2/* the expression is always false (not
225-
* implemented) */
226-
#defineV_STOP3/* the expression is a stop word */
227-
228210
/*
229-
* Remove QI_VALSTOP (stopword nodes) from query tree.
211+
* Remove QI_VALSTOP (stopword) nodes from query tree.
212+
*
213+
* Returns NULL if the query degenerates to nothing. Input must not be NULL.
214+
*
215+
* When we remove a phrase operator due to removing one or both of its
216+
* arguments, we might need to adjust the distance of a parent phrase
217+
* operator. For example, 'a' is a stopword, so:
218+
*(b <-> a) <-> c should becomeb <2> c
219+
*b <-> (a <-> c) should becomeb <2> c
220+
*(b <-> (a <-> a)) <-> c should becomeb <3> c
221+
*b <-> ((a <-> a) <-> c) should becomeb <3> c
222+
* To handle that, we define two output parameters:
223+
*ladd: amount to add to a phrase distance to the left of this node
224+
*radd: amount to add to a phrase distance to the right of this node
225+
* We need two outputs because we could need to bubble up adjustments to two
226+
* different parent phrase operators. Consider
227+
*w <-> (((a <-> x) <2> (y <3> a)) <-> z)
228+
* After we've removed the two a's and are considering the <2> node (which is
229+
* now just x <2> y), we have an ladd distance of 1 that needs to propagate
230+
* up to the topmost (leftmost) <->, and an radd distance of 3 that needs to
231+
* propagate to the rightmost <->, so that we'll end up with
232+
*w <2> ((x <2> y) <4> z)
233+
* Near the bottom of the tree, we may have subtrees consisting only of
234+
* stopwords. The distances of any phrase operators within such a subtree are
235+
* summed and propagated to both ladd and radd, since we don't know which side
236+
* of the lowest surviving phrase operator we are in. The rule is that any
237+
* subtree that degenerates to NULL must return equal values of ladd and radd,
238+
* and the parent node dealing with it should incorporate only one of those.
239+
*
240+
* Currently, we only implement this adjustment for adjacent phrase operators.
241+
* Thus for example 'x <-> ((a <-> y) | z)' will become 'x <-> (y | z)', which
242+
* isn't ideal, but there is no way to represent the really desired semantics
243+
* without some redesign of the tsquery structure. Certainly it would not be
244+
* any better to convert that to 'x <2> (y | z)'. Since this is such a weird
245+
* corner case, let it go for now. But we can fix it in cases where the
246+
* intervening non-phrase operator also gets removed, for example
247+
* '((x <-> a) | a) <-> y' will become 'x <2> y'.
230248
*/
231249
staticNODE*
232-
clean_fakeval_intree(NODE*node,char*result,int*adddistance)
250+
clean_stopword_intree(NODE*node,int*ladd,int*radd)
233251
{
234-
charlresult=V_UNKNOWN,
235-
rresult=V_UNKNOWN;
236-
237252
/* since this function recurses, it could be driven to stack overflow. */
238253
check_stack_depth();
239254

240-
if (adddistance)
241-
*adddistance=0;
255+
/* default output parameters indicate no change in parent distance */
256+
*ladd=*radd=0;
242257

243258
if (node->valnode->type==QI_VAL)
244259
returnnode;
245260
elseif (node->valnode->type==QI_VALSTOP)
246261
{
247262
pfree(node);
248-
*result=V_STOP;
249263
returnNULL;
250264
}
251265

252266
Assert(node->valnode->type==QI_OPR);
253267

254268
if (node->valnode->qoperator.oper==OP_NOT)
255269
{
256-
node->right=clean_fakeval_intree(node->right,&rresult,NULL);
270+
/* NOT doesn't change pattern width, so just report child distances */
271+
node->right=clean_stopword_intree(node->right,ladd,radd);
257272
if (!node->right)
258273
{
259-
*result=V_STOP;
260274
freetree(node);
261275
returnNULL;
262276
}
263277
}
264278
else
265279
{
266280
NODE*res=node;
281+
boolisphrase;
267282
intndistance,
268-
ldistance=0,
269-
rdistance=0;
283+
lladd,
284+
lradd,
285+
rladd,
286+
rradd;
270287

271-
ndistance= (node->valnode->qoperator.oper==OP_PHRASE) ?
272-
node->valnode->qoperator.distance :
273-
0;
288+
/* First, recurse */
289+
node->left=clean_stopword_intree(node->left,&lladd,&lradd);
290+
node->right=clean_stopword_intree(node->right,&rladd,&rradd);
274291

275-
node->left=clean_fakeval_intree(node->left,
276-
&lresult,
277-
ndistance ?&ldistance :NULL);
292+
/* Check if currentnode is OP_PHRASE, get its distance */
293+
isphrase= (node->valnode->qoperator.oper==OP_PHRASE);
294+
ndistance=isphrase ?node->valnode->qoperator.distance :0;
278295

279-
node->right=clean_fakeval_intree(node->right,
280-
&rresult,
281-
ndistance ?&rdistance :NULL);
282-
283-
/*
284-
* ndistance, ldistance and rdistance are greater than zero if their
285-
* corresponding nodes are OP_PHRASE
286-
*/
287-
288-
if (lresult==V_STOP&&rresult==V_STOP)
296+
if (node->left==NULL&&node->right==NULL)
289297
{
290-
if (adddistance&&ndistance)
291-
*adddistance=ldistance+ndistance+rdistance;
298+
/*
299+
* When we collapse out a phrase node entirely, propagate its own
300+
* distance into both *ladd and *radd; it is the responsibility of
301+
* the parent node to count it only once. Also, for a phrase
302+
* node, distances coming from children are summed and propagated
303+
* up to parent (we assume lladd == lradd and rladd == rradd, else
304+
* rule was broken at a lower level). But if this isn't a phrase
305+
* node, take the larger of the two child distances; that
306+
* corresponds to what TS_execute will do in non-stopword cases.
307+
*/
308+
if (isphrase)
309+
*ladd=*radd=lladd+ndistance+rladd;
310+
else
311+
*ladd=*radd=Max(lladd,rladd);
292312
freetree(node);
293-
*result=V_STOP;
294313
returnNULL;
295314
}
296-
elseif (lresult==V_STOP)
315+
elseif (node->left==NULL)
297316
{
317+
/* Removing this operator and left subnode */
318+
/* lladd and lradd are equal/redundant, don't count both */
319+
if (isphrase)
320+
{
321+
/* operator's own distance must propagate to left */
322+
*ladd=lladd+ndistance+rladd;
323+
*radd=rradd;
324+
}
325+
else
326+
{
327+
/* at non-phrase op, just forget the left subnode entirely */
328+
*ladd=rladd;
329+
*radd=rradd;
330+
}
298331
res=node->right;
299-
300-
/*
301-
* propagate distance from current node to the right upper
302-
* subtree.
303-
*/
304-
if (adddistance&&ndistance)
305-
*adddistance=rdistance;
306332
pfree(node);
307333
}
308-
elseif (rresult==V_STOP)
334+
elseif (node->right==NULL)
309335
{
336+
/* Removing this operator and right subnode */
337+
/* rladd and rradd are equal/redundant, don't count both */
338+
if (isphrase)
339+
{
340+
/* operator's own distance must propagate to right */
341+
*ladd=lladd;
342+
*radd=lradd+ndistance+rradd;
343+
}
344+
else
345+
{
346+
/* at non-phrase op, just forget the right subnode entirely */
347+
*ladd=lladd;
348+
*radd=lradd;
349+
}
310350
res=node->left;
311-
312-
/*
313-
* propagate distance from current node to the upper tree.
314-
*/
315-
if (adddistance&&ndistance)
316-
*adddistance=ndistance+ldistance;
317351
pfree(node);
318352
}
319-
elseif (ndistance)
353+
elseif (isphrase)
320354
{
321-
node->valnode->qoperator.distance+=ldistance;
322-
if (adddistance)
323-
*adddistance=0;
355+
/* Absorb appropriate corrections at this level */
356+
node->valnode->qoperator.distance+=lradd+rladd;
357+
/* Propagate up any unaccounted-for corrections */
358+
*ladd=lladd;
359+
*radd=rradd;
324360
}
325-
elseif (adddistance)
361+
else
326362
{
327-
*adddistance=0;
363+
/* We're keeping a non-phrase operator, so ladd/radd remain 0 */
328364
}
329365

330366
returnres;
@@ -585,7 +621,8 @@ cleanup_fakeval_and_phrase(TSQuery in)
585621
commonlen,
586622
i;
587623
NODE*root;
588-
charresult=V_UNKNOWN;
624+
intladd,
625+
radd;
589626
TSQueryout;
590627
QueryItem*items;
591628
char*operands;
@@ -594,8 +631,8 @@ cleanup_fakeval_and_phrase(TSQuery in)
594631
returnin;
595632

596633
/* eliminate stop words */
597-
root=clean_fakeval_intree(maketree(GETQUERY(in)),&result,NULL);
598-
if (result!=V_UNKNOWN)
634+
root=clean_stopword_intree(maketree(GETQUERY(in)),&ladd,&radd);
635+
if (root==NULL)
599636
{
600637
ereport(NOTICE,
601638
(errmsg("text-search query contains only stop words or doesn't contain lexemes, ignored")));

‎src/test/regress/expected/tsearch.out

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ SELECT to_tsquery('english', 'a <-> (1 <-> 2)');
594594
SELECT to_tsquery('english', '1 <-> (a <-> 2)');
595595
to_tsquery
596596
-------------
597-
'1' <-> '2'
597+
'1' <2> '2'
598598
(1 row)
599599

600600
SELECT to_tsquery('english', '1 <-> (2 <-> a)');
@@ -630,7 +630,7 @@ SELECT to_tsquery('english', 'a <3> (1 <-> 2)');
630630
SELECT to_tsquery('english', '1 <3> (a <-> 2)');
631631
to_tsquery
632632
-------------
633-
'1' <3> '2'
633+
'1' <4> '2'
634634
(1 row)
635635

636636
SELECT to_tsquery('english', '1 <3> (2 <-> a)');
@@ -666,7 +666,7 @@ SELECT to_tsquery('english', 'a <-> (1 <3> 2)');
666666
SELECT to_tsquery('english', '1 <-> (a <3> 2)');
667667
to_tsquery
668668
-------------
669-
'1' <-> '2'
669+
'1' <4> '2'
670670
(1 row)
671671

672672
SELECT to_tsquery('english', '1 <-> (2 <3> a)');
@@ -684,7 +684,7 @@ SELECT to_tsquery('english', '((a <-> 1) <-> 2) <-> s');
684684
SELECT to_tsquery('english', '(2 <-> (a <-> 1)) <-> s');
685685
to_tsquery
686686
-------------
687-
'2' <-> '1'
687+
'2' <2> '1'
688688
(1 row)
689689

690690
SELECT to_tsquery('english', '((1 <-> a) <-> 2) <-> s');
@@ -708,7 +708,7 @@ SELECT to_tsquery('english', 's <-> ((a <-> 1) <-> 2)');
708708
SELECT to_tsquery('english', 's <-> (2 <-> (a <-> 1))');
709709
to_tsquery
710710
-------------
711-
'2' <-> '1'
711+
'2' <2> '1'
712712
(1 row)
713713

714714
SELECT to_tsquery('english', 's <-> ((1 <-> a) <-> 2)');
@@ -750,13 +750,13 @@ SELECT to_tsquery('english', '(s <-> (1 <-> a)) <-> 2');
750750
SELECT to_tsquery('english', '2 <-> ((a <-> 1) <-> s)');
751751
to_tsquery
752752
-------------
753-
'2' <-> '1'
753+
'2' <2> '1'
754754
(1 row)
755755

756756
SELECT to_tsquery('english', '2 <-> (s <-> (a <-> 1))');
757757
to_tsquery
758758
-------------
759-
'2' <-> '1'
759+
'2' <3> '1'
760760
(1 row)
761761

762762
SELECT to_tsquery('english', '2 <-> ((1 <-> a) <-> s)');
@@ -768,13 +768,13 @@ SELECT to_tsquery('english', '2 <-> ((1 <-> a) <-> s)');
768768
SELECT to_tsquery('english', '2 <-> (s <-> (1 <-> a))');
769769
to_tsquery
770770
-------------
771-
'2' <-> '1'
771+
'2' <2> '1'
772772
(1 row)
773773

774774
SELECT to_tsquery('english', 'foo <-> (a <-> (the <-> bar))');
775775
to_tsquery
776776
-----------------
777-
'foo' <-> 'bar'
777+
'foo' <3> 'bar'
778778
(1 row)
779779

780780
SELECT to_tsquery('english', '((foo <-> a) <-> the) <-> bar');

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp