On 02/22/2017 12:00 PM, Josh Smeaton wrote: > - I'm unsure how DEFAULT_INDEX_TABLESPACE is causing issues with your > patch, can you explain a bit?
It appears that it's necessary to have functions defined django/db/models/functions/X.py be importable from django.db.models.functions, which means that they should be in django/db/models/functions/__init__.py. I tried to experiment with this, but I get a traceback about DEFAULT_INDEX_TABLESPACE not being set, which again comes back Field referencing settings.DEFAULT_INDEX_TABLESPACE. I had to revert the commit again: https://github.com/django/django/pull/7611/commits /0568902abadafa25fdc81b2ec5df50e5b7c11be7 > - Explicit arguments in functions are a nice to have, and definitely > help when you know exactly what should and shouldn't be allowed, but are > not required. For functions that take no arguments, it's better to > override the `__init__` method and make that explicit. You could also > define the `arity` class property if you know exactly how many > expression arguments are needed for a Func. The window functions are a bit of an odd case, as they are both applicable in X() OVER(), but also in X() WITHIN GROUP(ORDER BY ...) where X can take different arguments. I posted a couple links to some blogs that explain this with a few examples. I'd like them also to support WITHIN GROUP, where they can accept an argument; the output-field is always for some functions either an integer or a float, regardless of the input, and for others, such as last_value, it depends on the type of the first argument, i.e., last_value(column with a date-type) should output a date-type. https://www.depesz.com/2014/01/11/waiting-for-9-4-support-ordered-set-within-group-aggregates/ https://sqlandplsql.com/2012/04/18/oracle-rank-function/ > - __str__ and __repr__ methods don't need to and shouldn't return > database specific representations. They're there for debugging support > mostly, and users aren't going to care (too much) if the PRECEDING > language is slightly different from their particular backend. Hard code > what you think will be the most useful representation in the __str__ and > __repr__ methods. Tests are there to encourage implementors to add > str/repr methods to their own expressions, that is all. OK. Good. > - are there other cases that should fail? Perhaps aggregating a window > clause? I don't actually know, but it'd be worth testing. I think this > will be caught by virtue of window functions deriving from Aggregate, > but double check: > > WindowTestModel.objects.annotate(lead=Window( > expression=Lead('salary'), > partition_by='department') > ).aggregate(Sum('lead')) This doesn't crash on Postgres, and sums the result of lead together. It adds a group by, though. I suppose an exception should also be raised in case there's no ORDER BY inside the window, at least some of the tests failed on Oracle because they didn't specify the ordering. For instance, Oracle wasn't thrilled about ROW_NUMBER() OVER(), although Postgres could understand it (I haven't yet installed MariaDB locally to get an idea what works and what doesn't). Thank you. I got a few clues to work from here, but I'll ask again at some point (I spent quite a lot of time on this, so I want to see it merged) :) > Cheers -- Med venlig hilsen / Kind regards, Mads Jensen Saajan Fernandes: I think we forget things if there is nobody to tell them. -- The Lunchbox (2013) -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/7076baa1-78bd-c850-9abe-ed1346ad3e5b%40inducks.org. For more options, visit https://groups.google.com/d/optout.
