[Django] #29997: Allow combining SearchQuerys with different configs

6 views
Skip to first unread message

Django

unread,
Nov 29, 2018, 5:39:10 AM11/29/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: nobody
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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'`

--
Ticket URL: <https://code.djangoproject.com/ticket/29997>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Nov 29, 2018, 5:42:44 AM11/29/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jaap Roes):

* owner: nobody => Jaap Roes


--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:1>

Django

unread,
Nov 29, 2018, 10:45:44 AM11/29/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jaap Roes):

* type: Cleanup/optimization => Bug


Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:2>

Django

unread,
Nov 29, 2018, 12:53:26 PM11/29/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* stage: Unreviewed => Accepted


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:3>

Django

unread,
Nov 29, 2018, 12:53:50 PM11/29/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
----------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 1.11

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+-------------------------------------
Changes (by Simon Charette):

* version: master => 1.11
* component: Database layer (models, ORM) => contrib.postgres


--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:4>

Django

unread,
Nov 29, 2018, 3:07:59 PM11/29/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* has_patch: 0 => 1
* stage: Accepted => Ready for checkin


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:5>

Django

unread,
Nov 30, 2018, 3:14:30 AM11/30/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (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)

--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:6>

Django

unread,
Nov 30, 2018, 9:43:40 AM11/30/18
to django-...@googlegroups.com
#29997: Allow combining SearchQuerys with different configs
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: Jaap Roes
Type: Bug | Status: closed
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"8a1a9194686881e32d0e3aed52d7f4150238530c" 8a1a9194]:
{{{
#!CommitTicketReference repository=""
revision="8a1a9194686881e32d0e3aed52d7f4150238530c"
Fixed #29997 -- Allowed combining SearchQuerys with different configs.

Seems to be a needless restriction in
978a00e39fee25cfa99065285b0de88366710fad.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29997#comment:7>

Reply all
Reply to author
Forward
0 new messages