Added expression support for QuerySet.update

Some quick tests, not a ton, but tests basic funcionality, should be a good starting point if anyone wants to help out.

See http://groups.google.com/group/django-developers/browse_thread/thread/d150c28e9dfe07a7

would this patch allow ColumnExpr to be used in .filter() ?

Yes and exclude() too. But it is not called ColumnExpr? anymore. See the discussion at the mailing list (link above).

updated patch to r7631

fixed patch - last one was missing two files

Added 2 missing files.

Added 2 missing files.

I've read through this patch in detail for the first time. It smells a bit over-engineered in places, so I'm going to have to think about that (there seem to be too many extra classes involved). For now, though, there are some more fundamental problems I'd like to bring up:

1. You've somewhat arbitrarily removed the get_placeholder() stuff. That is there because it's needed by certain extensions (in particular, geo-django, but it will also be useful in other cases). Remember that this is all new code; it's not like things are hanging around for historical reasons. Basically the "placeholder" is either "%s" in the normal case or some other format string (e.g. "SOME_FUNC(%s)" in cases like the GIS situations. So treat the placeholder as an opaque string that you use instead of %s to indicate parameters.
2. The WhereNode class is now a proper tree of other WhereNode classes (after [7835]), so that might affect some things in this patch. In particular, converting things passed in to values should be done in WhereNode.add() so that no references to fields or models are stored in the WhereNode class. This avoids infinite loops and pickling problems.
3. Calling curry() in make_atom() doesn't looks useful. You just want to save it to use as a function later when you still have the pieces of information you use. Query construction takes long enough without extra overhead like this, so just call the function directly at the right moment.
4. Having to pass opts to as_sql() -- which will now be add() after [7835] -- feels wrong. The where-class itself shouldn't need to care about that, it's purely for the benefit of the "smart objects", so they should contain the information they need. This feeds into the next point (and below) that, in general, these classes should know how to convert themselves to SQL fragments.
5. Having to convert normal values to LiteralValues just to convert them back looks like unneeded overhead to me. Follow the pattern elsewhere: the default case will be normal values (the things we do now). That will be by far the most common stuff and it gets handled directly in make_atom(). Anything else, such as any smart objects, should convert themselves via a common method (their interface) and return the resulting string and list. They should have something like their own as_sql() or make_atom() method that is called if we detect the "value" has such a method. So it will be similar to the approach in as_sql(): if the thing we're processing has its own as_sql() method, call that; otherwise, it's a basic piece of data and we'll handle it via make_atom(). In this case with Expression derivatives, maybe make_atom() needs to check if value() has its own make_atom() or make_value() method or something. The idea here is to keep the level of coupling between the normal WhereNode code and any objects like F() to an absolute minimum. F and Expression don't need to be treated specially. They are examples of a class of smart objects that should know how to prepare themselves.

I realise points 4 and 5 above sounds a bit less concrete than the others, but they're actually pretty major. Right now, this patch introduces some pretty tight coupling between these new classes and the query construction code. My intuition is that this coupling is not necessary. If you have to mention something like Expression in WhereNode, you've probably got a leaky abstraction. Use the interface that something like Expression should have to call it and that way we're not tied to only using the Expression class.

I suspect portions of this could probably be broken up into separate steps. I like the idea of an F() object; that's certainly necessary. All this stuff to allow additions and subtraction and other manipulations of values might be useful at some point, but it's probably less important than the ability to refer to a field in an expression and working out the correct way to call smart objects as values. It can certainly be added later and, if the changes I'm talking about above are done right, doesn't have to be part of core immediately, which buys us some room to experiment. So don't try to do too much at once here.

One thought I had is that the bit in make_atom() that currently says

if isinstance(params, QueryWrapper):
   ...

could take over the general role of smart objects here and become something like

if hasattr(params, 'make_value'):
   ...

and then we have to work out how to weave general return values into the result (the current (extra, params) tuple probably won't be enough). In any case, that's the sort of lines I'd work along to make things work like the rest of the code throughout the query construction: we provide the means to "shell out" to advanced objects and handle only the base case in the core code.

Note that this shows we've already got this slightly leaky abstraction in the code, as a once-off to handle SQL subqueries as values. Designing this particular patch correctly should actually help plug that leak by possibly removing the need for WhereNode to know or care about QueryWrapper.

That's where I'm sitting at the moment. The idea is certainly worth pursuing. There are some implementation issues that need to be solved, as well as some broader design ones before we can go further. I'm not completely convinced it's "1.0-beta" material. Nice to have, but not a showstopper if we don't have it in 1.0. We can certainly add this stuff at any point without breaking the external API. Getting it right is therefore more important than getting it done quickly.

Another thing I've noticed with this patch is that it handles the fields based on the model's _meta attribute. This is a problem if F (or any expression) is going to be used with extra(), related fields or aggregates. I wanted to re-write it to avoid the dependency of the model at least for the quering case.

Also, the latest trunk revision introduces some changes that heavily conflict with this patch. Mostly the in "where.py" and "sql/query.py".

The only problem I see with the objects converting themselves to SQL fragments is that sometimes the objects don't have all the necessary information to do that and would require information of the state of the query class (e.g. aliases). The same problem would arrise if we want to allow relation spaning fields to be used without explicitly joining the tables beforehand. The second point (relation spaning fields) is a far fetched case and I'm not sure it should even be allowed, but the first one is a common case that could easily be solved by setting a placeholder for the field information and letting the queryset code handle the propper name selection.

I am not sure either that this belongs in "1.0-beta" but I have been working on it and would like to get it working after EuroPython's sprint.