Skip to content

Instantly share code, notes, and snippets.

@akaariai
Last active August 29, 2015 14:02
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save akaariai/3b78a6ae1d7cd694b36f to your computer and use it in GitHub Desktop.
Save akaariai/3b78a6ae1d7cd694b36f to your computer and use it in GitHub Desktop.
This is a heads-up post about some planned changes to the ORM and specifically to the expressions API. This affects how the following features are dealt with inside the ORM:
- F-expressions (and other ExpressionNodes)
- Aggregates
- Anything using SQLEvaluator (django.db.models.sql.expressions)
While the changes target private APIs, those APIs have remained stable for a long time and I expect there to be users of the above APIs. The main concern is that the planned changes will break existing code. We need feedback from users about how these changes will break existing code. If we find some common cases we might be able to add backwards compatibility code paths for those cases.
There are two main reasons for the change. First, the change allows for a lot of nice new features - doing conditional aggregates, aggregates using expressions and writing custom expressions, all this using public APIs. The second reason is that the current coding is somewhat complex, and that complexity makes it hard to write custom aggregates or expressions.
Currently the expressions and aggregates are built up from two classes - the first one is public facing API (for example Sum in django.db.models.aggregates), the second is how the public facing API is executed in the ORM (Sum in django.db.models.sql.aggregates). The idea is that we have one public facing component, and different Query implementations can have different implementation classes for the public facing component. Unfortunately this leads to cases where it is hard to extend expressions or aggregates - while it is easy to add a new public facing API class, it isn't easy to add an implementation for that class - the implementation belongs to the used Query class, but that class isn't under user control. In addition the current implementation is somewhat complex to follow. Still, aggregates had a separate implementation from expressions.
The new way is simplified - there is just public facing classes which know how to implement themselves. The new expressions know how to add themselves to the query, and they know how to generate a query against different database backends. Different database backends are handled with as_vendorname() syntax. Aggregates are a subclass of certain kind of expression (Func class), so aggregates use the same code paths as other expressions. The end results is simplified code, ability to use Sum('foo') + Sum('bar') style aggregations, and the ability to write new expressions and aggregates using a public stable API.
A patch exists that implements the new way. It is written by Josh Smeaton. The patch also implements a way to annotate non-aggregates to queries (.annotate(Coalesce(F('foo'), F('bar'))). The patch can be used as basis for other improvements to the ORM, for example the ability to queries like .order_by(Lower('somecol').desc()) has been discussed on this list recently.
The problems with introducing the new way is backwards compatibility. The current coding is implemented the way it is as the aim was allowing writing of "NoSQL" style backends. The NoSQLQuery just needs to contain different implementation class than the normal Query class had. I claim that it is possible to do the exact same thing with addition of a rewriter - it inspects the new-style classes, and creates different implementation classes on the fly.
The bigger problem seems to be existing 3rd party aggregates and expressions - while technically we are changing only private APIs, I don't see it as a good idea to break existing code if we can avoid that. So, if you depend on the current implementation, please check if the new way breaks your code. If so, report that in ticket #14030, and lets see if there is something we can do about that. Of course other feedback is also welcome.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment