Opened 4 months ago

Closed 4 months ago

#29997 closed Bug (fixed)

Allow combining SearchQuerys with different configs

Reported by: Jaap Roes Owned by: Jaap Roes
Component: contrib.postgres Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The fix for ticket #27143 (SearchQuery is not combinable using more than one & or | operators) introduced some code that raises a TypeError when trying to combine SearchQuery objects that do not share the same config.

I'm not sure if this restriction makes sense. In SQL it seems to be allowed e.g.:

SELECT to_tsquery('simple', 'working') || to_tsquery('english', 'working');
'working' | 'work'

Change History (7)

comment:1 Changed 4 months ago by Jaap Roes

Owner: changed from nobody to Jaap Roes
Last edited 4 months ago by Tim Graham (previous) (diff)

comment:2 Changed 4 months ago by Jaap Roes

Type: Cleanup/optimizationBug

I ran git bisect and technically this is a regression from commit 978a00e39fee25cfa99065285b0de88366710fad

Before that commit this works:

search.SearchQuery("foo", config="simple") | search.SearchQuery("bar", config="english")

Afterwards it doesn't

comment:3 Changed 4 months ago by Simon Charette

Triage Stage: UnreviewedAccepted

Thanks for the bisect sleuthing here. Given this is a regression in 1.11 I'm not sure this qualifies for a backport for 2.1 at this point.

comment:4 Changed 4 months ago by Simon Charette

Component: Database layer (models, ORM)contrib.postgres
Version: master1.11

comment:5 Changed 4 months ago by Tim Graham

Has patch: set
Triage Stage: AcceptedReady for checkin

Before merging, I'll give the original author some time to respond to the question about why the restriction was added. The patch looks fine.

comment:6 Changed 4 months ago by Jaap Roes

fwiw, I think the config equality check was copied from the implementation of SearchVectorCombinable. The validity of that check there is also questioned by at least one person (see ticket #28528)

comment:7 Changed 4 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 8a1a9194:

Fixed #29997 -- Allowed combining SearchQuerys with different configs.

Seems to be a needless restriction in
978a00e39fee25cfa99065285b0de88366710fad.

Note: See TracTickets for help on using tickets.
Back to Top