First off, great work on this patch. I haven't given it much of a look over
just yet unfortunately, but I'm trying to figure out where your issues are
to help unblock.
- I'm unsure how DEFAULT_INDEX_TABLESPACE is causing issues with your
patch, can you explain a bit?
- 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.
- Overriding resolve_output_field is a fine idea, and we should probably
make use of that a little better in other types. You could also just
override an `output_field` property to extract the output_field of the
first expression in self.get_source_expressions.
- django.db.models.functions.windows is fine
- __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.
- 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'))
Happy to clarify anything above if I wasn't too clear.
Cheers
On Tuesday, 21 February 2017 17:36:04 UTC+11, Mads Jensen wrote:
>
> I hit somewhat of a wall, and would like some input/help.
>
> * Import of window functions from `django.db.model.functions` seems to be
> the
> convention - `DEFAULT_INDEX_TABLESPACE` causes some headaches, because of
> XField being explicitly set in _output_field variable. Probably the
> functions need some extra fine-tuning in some way. So
> far, most of the functions don't have explicit arguments in the
> constructor,
> because they either have a varying number of arguments (Lead and Lag, for
> instance) or don't take any (rank and row_number; however, those can
> take an
> argument in case somebody wants to add support for WITHIN
> GROUP-expression). I
> deleted the "_output_field = Field" from the functions that do take an
> argument, such as Lead and Lag, would it be required to explicit about
> the
> output_field, such as overriding the resolve_output_field-field? In those
> cases, it's always the type of the first argument or the
> default-argument in
> case it's provided/available. Tests are running locally without the
> explicit
> _output_field. And just to avoid shuffling around things later, any
> arguments
> against django.db.models.functions.window being the right place for this?
>
> * `__repr__` and `__str__` methods and testing. What's a good general
> convention
> for something like this? I don't have access to the connection in
> __repr__,
> and __str__-methods, and thus just imported from `django.db` in the
> tests,
> which I suppose is not the ideal solution for this. At least, it doesn't
> really feel that way.
>
> * Are there more cases which should fail apart from filtering (supported by
> filterable) and use in UPDATE (can_be_reffed_in_update)?
>
> Thank you.
>
> Kind regards,
> Mads
>
--
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/5662f3d4-619c-49df-ab02-efdeb8ee2573%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
