Opened 9 years ago

Closed 5 years ago

Last modified 4 years ago

#25367 closed Cleanup/optimization (fixed)

Allow expressions in .filter() calls

Reported by: Anssi Kääriäinen Owned by: Matthew Schinckel
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: me@…, josh.smeaton@… 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

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.

Change History (19)

comment:1 by Tim Graham, 9 years ago

Keywords: 1.9 added

comment:2 by Adam Johnson, 9 years ago

Cc: me@… added

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.

comment:3 by Tim Graham, 9 years ago

Keywords: 1.9 removed

As noted on the pull request, there are some problems that can't be solved in time for the 1.9 freeze.

comment:4 by Matthew Schinckel, 7 years ago

I have a 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?

comment:5 by Matthew Schinckel, 7 years ago

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.

comment:6 by Matthew Schinckel, 7 years ago

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')?

comment:7 by Matthew Schinckel, 7 years ago

Owner: changed from nobody to Matthew Schinckel
Status: newassigned

comment:8 by Josh Smeaton, 7 years ago

Cc: josh.smeaton@… added
Triage Stage: AcceptedReady for checkin

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.

comment:9 by Tim Graham, 7 years ago

Needs documentation: unset
Triage Stage: Ready for checkinAccepted

After some reviews, the patch is still in progress.

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In f7e9db14:

Refs #25367 -- Added test for Exists() lookup rhs.

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In fff5186d:

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.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In efa1908f:

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.

comment:13 by Mariusz Felisiak, 5 years ago

Patch needs improvement: unset

comment:14 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 4137fc2e:

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 <hello@…>

comment:16 by GitHub <noreply@…>, 5 years ago

In d275fd04:

Refs #25367 -- Simplified OrderBy and Lookup by using Case() instead of RawSQL() on Oracle.

Follow up to efa1908f662c19038a944129c81462485c4a9fe8.

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 37e6c5b:

Refs #25367 -- Moved conditional expression wrapping to the Exact lookup.

comment:18 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In f97a6123:

Refs #25367 -- Made Query.build_filter() raise TypeError on non-conditional expressions.

comment:19 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In c2d49267:

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.

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