[Django] #25367: Allow expressions in .filter() calls
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 0
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Expressions in filter calls will allow 3rd party apps to create query
syntax extensions (for example .filter(F('some_field').lower() ==
'anssi')) style). In addition, having the ability to use expressions
everywhere unifies the ORM experience.
--
Ticket URL: <https://code.djangoproject.com/ticket/25367>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
(models, ORM) |
Severity: Normal | Resolution:
Keywords: 1.9 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: => 1.9
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: me@… (added)
Comment:
This would also probably allow `extra(where)` to be removed as discussed
on https://groups.google.com/forum/#!topic/django-developers/FojuU0syO8Y ,
if expressions with 0 column references are allowed.
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: 1.9 =>
Comment:
As noted on the pull request, there are some problems that can't be solved
in time for the 1.9 freeze.
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
I have a [https://github.com/django/django/pull/8119 PR] that addresses
part of this: being able to have an expression that returns a value with
an `output_field` of `BooleanField` used in a `.filter()`, or a
`Case(When(...))` situation.
Does this address enough of this ticket? Or provide any other suggestions?
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
The `.filter(F(x) == 'foo')` syntax doesn't work, and won't without quite
a bit more work (basically, I think this would require the `__eq__` method
on `F` to return a non-boolean, which I think is probably Bad News(tm).)
I think it can be spelled a different way using `ExpressionWrapper`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matthew Schinckel):
Actually, I'm not sure it can - however, I'm not sure that we actually
want to support that syntax - does it give anything over `Q(x='foo')`?
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Matthew Schinckel
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:7>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: josh.smeaton@… (added)
* stage: Accepted => Ready for checkin
Comment:
Supporting boolean expressions is enough for the framework. It will be
possible for 3rd party libs to support more fluent syntaxes by converting
algebraic expressions into database expressions, similar to how combinable
works. Whether or not anyone attempts to do that is neither here nor
there.
I think this is a really good change. Having to annotate to filter is not
just cumbersome, but as you've shown also a large performance hit.
The docs might need a little bit of work, but I think the code and tests
are fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:8>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_docs: 1 => 0
* stage: Ready for checkin => Accepted
Comment:
After some reviews, the patch is still in progress.
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:9>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"f7e9db14bbd39a393260d0a8b44aae80cbb65c81" f7e9db14]:
{{{
#!CommitTicketReference repository=""
revision="f7e9db14bbd39a393260d0a8b44aae80cbb65c81"
Refs #25367 -- Added test for Exists() lookup rhs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:10>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"efa1908f662c19038a944129c81462485c4a9fe8" efa1908f]:
{{{
#!CommitTicketReference repository=""
revision="efa1908f662c19038a944129c81462485c4a9fe8"
Refs #25367 -- Moved Oracle Exists() handling to contextual methods.
Oracle requires the EXISTS expression to be wrapped in a CASE WHEN in
the following cases.
1. When part of a SELECT clause.
2. When part of a ORDER BY clause.
3. When compared against another expression in the WHERE clause.
This commit moves the systematic CASE WHEN wrapping of Exists.as_oracle
to contextual .select_format, Lookup.as_oracle, and OrderBy.as_oracle
methods in order to avoid unnecessary wrapping.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:12>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fff5186d3215e0ba06e47090226169f2230786b0" fff5186d]:
{{{
#!CommitTicketReference repository=""
revision="fff5186d3215e0ba06e47090226169f2230786b0"
Refs #25367 -- Moved select_format hook to BaseExpression.
This will expose an intermediary hook for expressions that need special
formatting when used in a SELECT clause.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:11>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:13>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:14>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"4137fc2efce2dde48340728b8006fc6d66b9e3a5" 4137fc2e]:
{{{
#!CommitTicketReference repository=""
revision="4137fc2efce2dde48340728b8006fc6d66b9e3a5"
Fixed #25367 -- Allowed boolean expressions in QuerySet.filter() and
exclude().
This allows using expressions that have an output_field that is a
BooleanField to be used directly in a queryset filters, or in the
When() clauses of a Case() expression.
Thanks Josh Smeaton, Tim Graham, Simon Charette, Mariusz Felisiak, and
Adam Johnson for reviews.
Co-Authored-By: NyanKiyoshi <he...@vanille.bid>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:15>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"d275fd04f3fbb1a6cfa2273feb6cb414ce143b8c" d275fd04]:
{{{
#!CommitTicketReference repository=""
revision="d275fd04f3fbb1a6cfa2273feb6cb414ce143b8c"
Refs #25367 -- Simplified OrderBy and Lookup by using Case() instead of
RawSQL() on Oracle.
Follow up to efa1908f662c19038a944129c81462485c4a9fe8.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:16>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"37e6c5b79bd0529a3c85b8c478e4002fd33a2a1d" 37e6c5b]:
{{{
#!CommitTicketReference repository=""
revision="37e6c5b79bd0529a3c85b8c478e4002fd33a2a1d"
Refs #25367 -- Moved conditional expression wrapping to the Exact lookup.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:17>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"f97a6123c07de5099fdf8b7d00ef7d20ed354e07" f97a6123]:
{{{
#!CommitTicketReference repository=""
revision="f97a6123c07de5099fdf8b7d00ef7d20ed354e07"
Refs #25367 -- Made Query.build_filter() raise TypeError on non-
conditional expressions.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:18>
Django
-------------------------------------+-------------------------------------
Type: | Schinckel
Cleanup/optimization | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"c2d4926702045e342a668057f0a758eec9db9436" c2d49267]:
{{{
#!CommitTicketReference repository=""
revision="c2d4926702045e342a668057f0a758eec9db9436"
Fixed #31910 -- Fixed crash of GIS aggregations over subqueries.
Regression was introduced by fff5186 but was due a long standing issue.
AggregateQuery was abusing Query.subquery: bool by stashing its
compiled inner query's SQL for later use in its compiler which made
select_format checks for Query.subquery wrongly assume the provide
query was a subquery.
This patch prevents that from happening by using a dedicated
inner_query attribute which is compiled at a later time by
SQLAggregateCompiler.
Moving the inner query's compilation to SQLAggregateCompiler.compile
had the side effect of addressing a long standing issue with
aggregation subquery pushdown which prevented converters from being
run. This is now fixed as the aggregation_regress adjustments
demonstrate.
Refs #25367.
Thanks Eran Keydar for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25367#comment:19>