Re: Improving aggregate support (#14030)

Code complete: https://github.com/django/django/pull/2496 - Aggregates have learnt arithmetic and how to interact with F() expressions - Annotations can now accept non-aggregate expressions I'm in the process of writing documentation, some of which I've committed and is available within the PR.

Re: Improving aggregate support (#14030)

There are a few other things that I need to try and get in before the 1.7 feature freeze. Since this is not slated for 1.7, I'll take a look a the PR after the freeze and see if the comments I had still apply. Regards, Michael Manfre On Sun, Jan 19, 2014 at 8:00 AM, Josh Smeaton wrote: > I've f

Re: Improving aggregate support (#14030)

I've finally sent a PR, so if you're still able, I'd love to see the specific comments you had. https://github.com/django/django/pull/2184 Cheers, - Josh On Sunday, 12 January 2014 10:35:58 UTC+11, Michael Manfre wrote: > > With minor tweaks to django-mssql's SQLCompiler, I am able to pass the

Re: Improving aggregate support (#14030)

That's fine, I knew I was cutting it close when I started. At least I can start getting to bed at a reasonable time now :) I was wondering where the dev Irc channel was, so I'll make use of that too. Thanks for the pointers so far. - Josh > On 16 Jan 2014, at 19:36, Anssi Kääriäinen wrote: >

Re: Improving aggregate support (#14030)

I'm sorry I haven't had enough time to review your work. Unfortunately I won't have any more time in the near future either. So, it looks like this has to wait for 1.8. Sorry for that. If you want to ask specific questions, #django-dev IRC channel is a good place to ask those questions. It is o

Re: Improving aggregate support (#14030)

Quick update. GIS support is nearly done, I just have two failing tests to take care of (one to do with lookups setting srid, and another that is producing the wrong value). I have a few validation checks to make inside prepare, and then it's on to writing tests that make use of aggregates and

Re: Improving aggregate support (#14030)

I've also written a basic hack (there's no other word for it really) that should support the majority of custom aggregates out in the wild. The exact commit is here: https://github.com/jarshwah/django/commit/74c945db2e12444dd012cf89dbf65dddce84ba7b Basically, it checks to see if add_to_query ex

Re: Improving aggregate support (#14030)

Vendor specific SQL is now possible, and uses the monkey patching described above. I worked around the problem of temporarily changing the template, by putting the template and the function into the extra dict on init. Any modifications made to the dict are then made for that instance only. Com

Re: Improving aggregate support (#14030)

I've rebased off your current lookups pull request to drag in all the changes made recently, and I pulled against upstream/master which has made looking at the diff between our branches extremely painful. Once your PR is applied to master, I'll be able to submit a proper PR though (after some s

Re: Improving aggregate support (#14030)

On Sunday, January 12, 2014 10:52:03 AM UTC+2, Josh Smeaton wrote: > > Any idea how to do this? >> >> The new way in custom lookups is trying to first call as_vendorname(qn, >> connection), if that doesn't exist then call the standard implementation, >> that is as_sql(). For example on sqlite Dja

Re: Improving aggregate support (#14030)

> > Any idea how to do this? > > The new way in custom lookups is trying to first call as_vendorname(qn, > connection), if that doesn't exist then call the standard implementation, > that is as_sql(). For example on sqlite Django will first try to call > as_sqlite(), if that doesn't exist then

Re: Improving aggregate support (#14030)

On Saturday, January 11, 2014 10:06:14 AM UTC+2, Josh Smeaton wrote: > > >- Custom backend implementation of aggregates and annotations > > Any idea how to do this? The new way in custom lookups is trying to first call as_vendorname(qn, connection), if that doesn't exist then call the standar

Re: Improving aggregate support (#14030)

Hi Michael, Thanks for taking a look, much appreciated. I haven't created a PR yet because I've rebased on top of Ansii's Lookups refactor which hasn't hit Master yet (and is changing slightly based on feedback of his PR). To answer your questions: Why were the aggregate specific fields, prope

Re: Improving aggregate support (#14030)

With minor tweaks to django-mssql's SQLCompiler, I am able to pass the aggregation and aggregation_regress tests. If you create the pull request, comments can be attached to specific lines of code. I haven't had time to do a full review of your changes, but my initial questions and comments are:

Re: Improving aggregate support (#14030)

A milestone has been reached. The diff is here: https://github.com/jarshwah/django/compare/akaariai:lookups_3...aggregates-14030?expand=1 - Existing annotations and aggregations working as they were - Complex annotations and complex aggregations now work - Complex annotations can now be

Re: Improving aggregate support (#14030)

I'll preface by saying it's late for me, and I'm about to sign off, so any mistakes below I will correct tomorrow after some sleep. > To write a custom aggregate one needed two classes, one for user facing > API, one for implementing that API for SQL queries. I've taken note of this already,

Re: Improving aggregate support (#14030)

On Friday, January 3, 2014 6:34:13 PM UTC+2, Josh Smeaton wrote: > > I now have all tests passing on Postgres and SQLite (See > here). > > I still haven't touched GIS, but I believe it should be as simple

Re: Improving aggregate support (#14030)

An update. I've had some success with refactoring Aggregates as ExpressionNodes. The aggregates test suite is now passing, but I still have some failures with aggregates_regress test suite. The refactoring should allow behaviour like below possible: Model.objects.annotate(total_time=Sum( F('tim

Re: Improving aggregate support (#14030)

I now have all tests passing on Postgres and SQLite (See here). I still haven't touched GIS, but I believe it should be as simple as changing the query to directly call prepare on the expression, and remo

Re: Improving aggregate support (#14030)

On Wednesday, December 25, 2013 5:52:10 PM UTC+2, Josh Smeaton wrote: > > So I've gone ahead and tried to remove SQLEvaluator by pushing its > functions down into ExpressionNode. For the most part, it's working well. I > rebased on akaariai/lookups_3 to make sure that any changes to lookups > wo

Re: Improving aggregate support (#14030)

So I've gone ahead and tried to remove SQLEvaluator by pushing its functions down into ExpressionNode. For the most part, it's working well. I rebased on akaariai/lookups_3 to make sure that any changes to lookups would be reflected in this change as they are *somewhat* related. The diff betwee

Re: Improving aggregate support (#14030)

So, after hacking away a little bit yesterday, I came to the same conclusion that Anssi did back when the first PR was sent. Namely, that the ExpressionNode <-> SQLEvaluator structure seems overly complex, and makes it difficult to create custom "ExpressionNodes". To build a custom Expression,

Re: Improving aggregate support (#14030)

Thanks for the response. I've noticed the JoinPromoter recently, and I have been looking into the custom lookups, but wasn't sure what the purpose was until now. I'll see what I can do with this patch, and try to avoid as much complexity as I can. Then it just comes down to review on whether it

Re: Improving aggregate support (#14030)

On Saturday, December 21, 2013 3:15:12 AM UTC+2, Josh Smeaton wrote: > > I'm interested in tackling > ticket#14030, > ideally before the Jan 20th cutoff, but I'm looking for some guidance. > There have been a lot of references to Anssi Kääriäinen atte

Improving aggregate support (#14030)

I'm interested in tackling ticket#14030, ideally before the Jan 20th cutoff, but I'm looking for some guidance. There have been a lot of references to Anssi Kääriäinen attempting an ORM refactor before trying to fix aggregates, but I'm not sure if th