Re: Improving aggregate support (#14030)

2014-03-30 Thread Josh Smeaton
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)

2014-01-19 Thread Michael Manfre
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

Re: Improving aggregate support (#14030)

2014-01-19 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2014-01-16 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2014-01-16 Thread Anssi Kääriäinen
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

Re: Improving aggregate support (#14030)

2014-01-15 Thread Josh Smeaton
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)

2014-01-14 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2014-01-13 Thread Josh Smeaton
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.

Re: Improving aggregate support (#14030)

2014-01-13 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2014-01-12 Thread Anssi Kääriäinen
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

Re: Improving aggregate support (#14030)

2014-01-12 Thread Josh Smeaton
> > 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)

2014-01-12 Thread Anssi Kääriäinen
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

Re: Improving aggregate support (#14030)

2014-01-11 Thread Josh Smeaton
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,

Re: Improving aggregate support (#14030)

2014-01-11 Thread Michael Manfre
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)

2014-01-11 Thread Josh Smeaton
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)

2014-01-08 Thread Anssi Kääriäinen
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)

2014-01-06 Thread Josh Smeaton
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(

Re: Improving aggregate support (#14030)

2014-01-03 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2013-12-28 Thread Anssi Kääriäinen
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 >

Re: Improving aggregate support (#14030)

2013-12-25 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2013-12-23 Thread Josh Smeaton
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)

2013-12-22 Thread Josh Smeaton
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

Re: Improving aggregate support (#14030)

2013-12-22 Thread Anssi Kääriäinen
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