Custom GROUP BY method. OLAP functionality.

[Mads Jensen]

Hello,

I started this rather messy draft (outline of how I think it could be 
approached), as I posted this ticket [1] some years ago. 
Basically, SQL adds some modifiers to group the result set in different ways. 
The ticket contains some links with good examples.

In order to solve this, it seems necessary to make a method that bypasses the 
group by logic of Django, so expert users can 
handle this themselves. Most likely, users who want to use this feature, are 
already more familiar with how a database works, 
than Django deals with GROUP BY quite well, so the name may be poorly chosen, 
because seasoned 
database people starting off with Django may be tempted to neglect this fact, 
and jump straight off to that method, not reading 
the admonition.

There's already get_group_by_cols which may be considered as well. I left a 
flag in the draft, Query.custom_group_by but haven't 
thought this through.

I think it should be manageable for me to work on getting subquery (I posted 
another thread 2-3 weeks ago) and a custom grouping 
method introduced in Django 3.2.

[1] https://code.djangoproject.com/ticket/27646
--
Med venlig hilsen / Kind regards,
Mads Jensen


-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/A6DC48D3554543B2BC0DBB8D175F90A7.MAI%40webhot.dk.

Filtering window expressions. Ticket #28333

[Mads Jensen]

Hey,

I originally submitted the window functions expressions to Django a couple of 
years ago for Django 2.0. In hindsight, 
there are probably things that could be implemented a bit better although I'm 
not sure how I easily could have used the
SQLCompiler or QuerySet/Query classes directly, especially considering the fact 
that there are slight discrepencies in
how window expressions are supported between the different databases that 
Django officielly supports.

Anyway, it spawned a ticket #28333 [1] that I like to dedicate some time to 
implement since the feature appears to be wanted. 

I have some thoughts about the process:

1. Include another flag: filtering_requires_subquery (should default to False, 
just filterable defaults to True)
2. Edit in QuerySet._filter_or_exclude to add somehow the following logic:

   1. If the expression is not filterable, then we can introduce a subquery and 
filter directly there.
  I surmise that this statement would be generic for SQL: if something is 
not filterable in a main query,
  then it will be including it in another query as a subquery, and 
filtering that way?
   2. Everything related to existing ordering etc. is a bit blurry.
   3. Having to work with table aliases doesn't seem to be something that I'd 
want to worry about.
  There's Query.rewrite_cols for the case when there's an annotation from a 
joined table,
  Sum(F('author__awards')) is the example given in the comment in that 
function.

3. I think it's a general lacune that it's difficult to add a subquery to an 
existing query.
   Although there's a boolean flag Query.subquery, it doesn't seem to make it 
easier to deal with filtering and other
   things related to a subquery. I found this when searching for tickets that 
have "subquery" in the description: 
   https://code.djangoproject.com/ticket/20127 

When are the table aliases used (T1, T2 etc.), and when are the actual table 
names used? I think it's when a table is auto-joined?

I'm not sure how much work that is required in SQLCompiler and Query, or if 
those two parts can stay untouched.

I tend to overthink things and sometimes make them more complex, so it's likely 
that there are things in the above that  

Query.as_sql has this:

def as_sql(self, compiler, connection):
sql, params = self.get_compiler(connection=connection).as_sql()
if self.subquery:
sql = '(%s)' % sql
return sql, params

If someone is willing to discuss ideas about the process, please reach out to 
me. Devil lies in the details, and I suppose some 
of the problem that subqueries are difficult in the ORM could be mended with 
work on this patch. However, I'd like to interact with
people who have worked more with the internals of the ORM than I did. One 
particular name Simon Charettes comes to mind, 
since he also volunteered to mentor GSOC in the ORM area.  

[1] https://code.djangoproject.com/ticket/28333
--
Med venlig hilsen / Kind regards,
Mads Jensen








-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3E827E5B921E41BF9E061DBDDF042F4F.MAI%40webhot.dk.

Re: GSoC 2020

[Mads Jensen]

On Monday, December 23, 2019 at 3:58:36 PM UTC+1, Nishchal Gupta wrote:
>
> Hello everyone,
>   
> This is Nishchal Gupta and I am current studying in KCNIT under (AKTU). I 
> have just completed my third semester. 
>
> I want to participate in Google Summer of Code 2020. I am familiar with 
> Django framework and don't know the steps to be taken.
>
> Also I don't have any idea whether the Organisation Django will 
> participate in 2020 or not.  Also I am not familiar with ticketing and unit 
> testing.
>
> Can anybody guide me the step by step process in order to get selected in 
> for GSoC 2020?
>

Please see the guidelines: 
https://docs.djangoproject.com/en/dev/internals/contributing/ as pointed 
out.

Your odds of being accepted will improve if you send PRs for some tickets. 
Another "low-hanging fruit" that you may consider to familiarize yourself 
with the code-base is increasing the test coverage:
https://djangoci.com/view/%C2%ADCoverage/
--
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/926c85ab-3c14-41f9-bf74-931434df0816%40googlegroups.com.

Partial indexes, PR help

[Mads Jensen]

Hi,

I've opened a PR to add support for partial indexes in Django. It's on 
the right track, but I bumped into an issue that I need help with. On 
PostgreSQL, the cast that's introduced when doing something like:


Q(published__gt=datetime.date(2017, 10, 1))
=>
"table"."published" > '2017-10-01'::timestamp;

is unfortunate because it turns the function mutable, and the index can 
therefore not be created (exact error message can be seen in the Jenkins 
output). I think that's a plausible use-case for a partial index to 
limit on dates. Locally, I have two tests that filters on the "id" (I 
didn't see any reason to introduce a new integer column in either of the 
existing models; using "pk" instead raised a warning about the column 
not being present) and one that filters on a boolean. Both were 
suggestions from Markus Holtermann on what he had seen used in 
production. There was already something for text strings.


My problem is that I'm a bit in the wild how to battle the lookup-cast 
issue on PostgreSQL. I've already tried to study the Lookup-flow. I see 
two ways:


1. Describe this short-coming well in the documentation, and leave it 
like this.
2. Introduce a flag that can trigger the cast in this special case. I 
didn't check to see how much of this that's public API and how much that 
isn't. It's what I'd favour, but also more difficult.


There's enough time to the feature freeze of Django 2.2 to have the 
first option as a last resort.


The tests are still failing, and Oracle had some transaction problem, 
that I'm also very oblivious about. Again, error messages are in the 
Jenkins output.


Link to the PR: https://github.com/django/django/pull/10140
--
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/c465d23f-0090-ee6b-98b5-9b1202beda39%40inducks.org.
For more options, visit https://groups.google.com/d/optout.

Re: Window expressions, #26608

[Mads Jensen]

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.

Re: Window expressions, #26608

[Mads Jensen]

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/d81ddfe0-f8f2-4ec0-99c9-96cd4b619246%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: CITextField base class

[Mads Jensen]

I had the use case for shorter text strings in mind when I did the PR.

https://github.com/atombrella/django/commit/ae8fe50f34329cbc969c5ac5bd368efd5b42b177
 
<- would this be a better approach?

On Wednesday, February 8, 2017 at 6:09:26 PM UTC+1, Tim Graham wrote:
>
> I thought the mixin idea was good but everyone else (including Aymeric) 
> reviewed the pull request and ignored your comment about that so I assumed 
> no one else saw its merits. Considering a CITextField that uses CharField 
> still has some usefulness as you mentioned, I'm in favor of CITextField not 
> inheriting anything and requiring its use as a mixin -- unless there are 
> some unforeseen problems with that.
>
> On Wednesday, February 8, 2017 at 11:55:50 AM UTC-5, Jon Dufresne wrote:
>>
>> > Does it make sense to add a CICharField for these cases or you can just 
>> override the form/admin.
>>
>> In a past Django developers discussion, Aymeric suggested building this 
>> functionality with a mixin so the citext type could apply to multiple DB 
>> fields which would then affect how the form field is rendered. Personally, 
>> I like this idea and believe it would provide the most flexibility such 
>> that a TextField, CharField, EmailField, etc. could all be backed by the 
>> citext type.
>>
>> https://groups.google.com/d/msg/django-developers/SW7_qI81G58/i6aJVRnJBAAJ
>>
>> From Aymeric in the previous thread:
>>
>> ---
>> I'm forseeing a small difficulty in terms of API. This is a behavior I'd 
>> like
>> to "mix in" to some fields but I can't say if that will be easy to 
>> implement.
>>
>> The general ideas would be:
>>
>> # A mixin
>>
>> class CITextField(TextField):
>> # ...
>>
>> # Case-insensitive versions of some built-in Django default fields
>> # (if we consider that makes sense)
>>
>> class CIEmailField(CITextField, EmailField):
>> pass
>>
>> # The possibility for users to make custom fields case insensitive
>>
>> class CITagField(CITextField, TagField):
>> pass
>> ---
>>
>

-- 
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/e878965e-51a2-443f-b158-57e44462cfe8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: CITextField base class

[Mads Jensen]

On Wednesday, February 8, 2017 at 11:32:22 AM UTC+1, Adam Johnson wrote:
>
> Sounds legit, make a ticket
>

Shouldn't https://code.djangoproject.com/ticket/26610 just be reopened?
 

> On 8 February 2017 at 00:37, Sean Brant  
> wrote:
>
>> I noticed the new postgres citext[1] field is a subclass of CharField. 
>> Wouldn't it be more correct to subclass TextField? Subclassing CharField 
>> means we need to add a max_length which is ignored in the db.
>>
>> [1] 
>> https://github.com/django/django/blob/master/django/contrib/postgres/fields/citext.py
>>
>> -- 
>> 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-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@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/0e50eae1-8806-47fc-b223-9cac7152bf1e%40googlegroups.com
>>  
>> 
>> .
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
>
> -- 
> Adam
>

-- 
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/e2be5fd6-3fc0-44d5-8d07-da713eb58db4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Window expressions, #26608

[Mads Jensen]

I think this has cleaned up to request input (I'm a bit zealous to make 
something that will be merged). I left a note in 1.11.txt but when 2.0.txt 
is introduced, I'll move it there. Also, now everything is in places that 
are not PostgreSQL-only. I realized in the process that the only thing the 
list of functions in the features-list of a backend is to prevent the test 
suite to run tests for unsupported functions. Mostly because MariaDB won't 
include FIRST_VALUE and LAST_VALUE and a few more. Perhaps a single list 
would be better?

I have a few questions to clear this up before asking for a more formal 
review:

1. PostgreSQL and MariaDB don't include support for RESPECT|IGNORE NULLS 
for certain functions (details are provided in
https://www.postgresql.org/docs/current/static/functions-window.html). Any 
thoughts if adding ignore_nulls=False|True to the constructor would be an 
idea, and then specify it's only possible for Oracle, or "as_oracle" with a 
special template? I noticed that nulls_last and nulls_first was added to 
OrderBy, however that is valid in all implementations. The Oracle virtual 
box was too big to fit on my machine, so I haven't tested locally for more 
than PostgreSQL, and on Python 3.

2. About __repr__ and __str__, in general, I'm in doubt what would be good 
ways to represent things, and how to test them? Also, I am in doubt what 
are intuitive input values that translate to CURRENT ROW, UNBOUNDED 
FOLLOWING etc.? I had a look at sqlalchemy which uses a mix of null-values 
and integers as valid input. In general, the frames-part is a bit of 
headache, and still slightly messy (the generated SQL is fine, so I suppose 
the current test data is adequate).

And probably, django/db/backends/mysql/features.py should be left untouched?

I know 1.11 is in the works, so I'm fine with this being on hold.

Thank you,
~ 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/822595ee-e75e-4979-8873-7b6488ee2d31%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Window expressions, #26608

[Mads Jensen]

On Tuesday, November 29, 2016 at 11:04:49 PM UTC+1, Josh Smeaton wrote:
>
> Adam, are you thinking we should be adding something like 
> Model.objects.window(), or just allowing Window-type expressions on 
> backends that have a specific feature flag? Does the compiler need to get 
> involved at all, or can we handle the full range of window expressions with 
> the expressions API?
>

I'd prefer the expression-api. I think it's more transparent what happens, 
and unless you're thinking of stating the windows explicitly after 
FROM/GROUP BY/HAVING, I don't see much reason.

I'm fairly new, but it seems to complicate things a lot more to need to 
write code inside the compiler, unless stricly necessary (supporting 
grouping sets could be one such example).
 

> Mads, there's nothing that currently handles a list of expressions, and 
> certainly nothing specific to OrderBy. Your proposed syntax is basically 
> what would be required `order_by=[ .. ]`.
>

I actually like the idea of introducing ExpressionList. It would provide a 
bit more power, such as offering ASC/DESC for each ordering expression. 
Anyway, I'll work a bit with this, and post some 
commits in the weekend. 

~ 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/b78f508c-b3b2-456c-b88a-ee26dead4aae%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Window expressions, #26608

[Mads Jensen]

On Tuesday, November 29, 2016 at 10:35:34 PM UTC+1, Adam Johnson wrote:
>
> As to your point 1:
>  
>
>> Since this is specific to postgres
>
>
> Well, Window expressions aren't specific to Postgres:
>
>- They exist in Oracle
>- They exist in MariaDB 10.2 which is in beta and will be GA at around 
>the same time this patch is out: 
>https://mariadb.com/kb/en/mariadb/window-functions/
>
> I thus think this should all be part of Django's main QuerySet API.
>

This is also my plan. I left out an important "not" in that particular 
phrase. If you notice, I also added feature-flags. But still, the code is 
quite messy.

~ 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/4ff78da9-0265-4aeb-be37-eb456fdc13e6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Window expressions, #26608

[Mads Jensen]

On Saturday, November 26, 2016 at 1:51:05 AM UTC+1, Josh Smeaton wrote:
>
> OrderBy takes an expression - it doesn't have to be a single column. For 
> example, this is valid:
>
> (Lower('last_name') + Lower('first_name)).desc() == 
> OrderBy(Lower('last_name') + Lower('first_name), descending=True)
>

Yes. However, I was more interested in being able to add a list of 
expressions. The OrderBy-class does not evaluate more than
one expression. The Case-expression right above iterates over all of the 
cases, and parses them. This is what I had in mind. I don't writing 
something for it, but
I'm trying to use as much of the existing API as possible.

What I had in mind was offering the flexibility to write this (maybe 
offering a list of OrderBy-clauses as valid input would be a way to 
remediate this):

I read over the Query-class code which has add_ordering.

qs = WindowTestModel.objects.annotate(sum=Window(
expression=Sum('salary'),
order_by=[ExtractYear('hiredate'), F('department')])
)
print(qs.query)

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/5aecd2cc-452c-43df-a0eb-b482220839eb%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Window expressions, #26608

[Mads Jensen]

On Tuesday, November 22, 2016 at 11:50:17 PM UTC+1, Josh Smeaton wrote:

> Thanks for picking this up. I've been wondering if Window expressions 
> would be possible, and what limitations we might have to make based on our 
> ORM.
>

I was fortunate that the reporter posted a snippet to work from :) Django's 
ORM is quite flexible, and adaptable. I remember seeing a video with Alex 
Gaynor where he complained about its shortcomings, but that seemed to have 
been sorted out.

https://www.youtube.com/watch?v=GxL9MnWlCwo
 

> > 1. Since this is specific to postgres, I'm looking for a better place to 
> put the actual Window-expression class, as well as axillary helpers 
> (Order By, Partition and the frame). 
>
> I would create a new file beside expressions.py called 
> django/db/models/window.py.
>
>
Seems like a good starting point. I have been contemplating that 
grouping.py could be a good name as well, in case somehow would like to add 
WITHIN GROUP, or support for grouping sets (a bigger fish, since that means 
appending stuff after GROUP BY etc.)
Anyway, I have some window.py locally.
 

> > 2. Tests, and if the initial test-cases seem fair? I took a small sample 
> of test cases related to some made-up salaries in a few departments.
>
> Yes, it's a good start, but I haven't thoroughly reviewed them yet either. 
> It'll be easier to do when there's a proper PR.
>

I wouldn't call it proper, but I posted a pull request, and marked it [WIP].

> 3. The Order By-clause proposed in the initial code in the code only 
> leaves room for one column (no expressions).
>
> Wouldn't the existing OrderBy type be sufficient? Really though, windowing 
> should support a list of expressions (then call asc() or desc() on them).
>

Maybe I'm reading the code wrong, but doesn't OrderBy just refer to a 
column, or are types of Expression/ExpressionWrapper also valid input?
 

> > 4. Would it be ok to add code for this in the base backend?
>
> Yes, if it makes the implementation nicer/easier for django and 3rd party 
> backends. If having the code in the backend prevents *users* from creating 
> their own windowing functions or modifying existing ones, then that's 
> probably not ok. Django, backends, and users all need to be able to 
> construct their own windowing expressions. You can certainly share code in 
> the backend though, with things like PRECEDING ROW etc.
>
 
I'm cleaning up things in the pull request. At least elements that are 
mandatory are there, such as docs, tests and some code.
 

> > I'm relatively new to Django internals (it's fun and educational hacking 
> on them), and it took me some time to read through the code and 
> documentation for the ORM, and there are many facets of it I can still 
> understand.
>
> None of us understand all of the ORM, and we're more than happy to have 
> more people helping out, so thanks! If you create a PR (even if it's not 
> ready, just mark is [WIP] for Work In Progress) it'll be easier to comment 
> on specific things within the code and see all of the commits tied 
> together. You're also more likely to get feedback on a PR and some guidance 
> there too.
>

Good. I tend to have several small things in my commits, but I'll try to 
work on individual things in this PR.

Thanks for input.

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/843eb618-7e8a-411e-aaa9-4efe5edce481%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Window expressions, #26608

[Mads Jensen]

Hi,

I got somewhat stuck on progress with this ticket, and as I'd like to
get it merged eventually (and avoid an abundant amount of fixes), I have
a few things I like a bit of input about.

1. Since this is specific to postgres, I'm looking for a better place to
put the actual Window-expression class, as well as axillary helpers
(Order By, Partition and the frame).
django/db/models/sql/expressions.sql may be one such place, but perhaps
introducing another file for this? Eventually, something like WITHIN
GROUP could be added, which shares some traits with window expressions.
This also goes the tests.

2. Tests, and if the initial test-cases seem fair? I took a small sample
of test cases related to some made-up salaries in a few departments. I
tried to introduce some wrapper to guard against testing functions that
aren't available (Oracle is the most complete in this sense, PostgreSQL
doesn't support all of the aggregate-functions, such as Variance).

3. The Order By-clause proposed in the initial code in the code only
leaves room for one column (no expressions). A regular query has
"add_ordering" to add more than one ordering-clause to a query, but this
just works for an ordering in SELECT, and I didn't spot anything that
could be used for this. Maybe abstracting the code from add_ordering so
it could be used also for something more? Something similar goes for
Partition By. Again, I'm sure it's a common use case to partition and
order by more than one expression. I currently commented out the
OrderByWrapper.

4. Would it be ok to add code for this in the base backend? I'm thinking
of constants for CURRENT ROW etc. which are shared among backends. I had
a look at SQLalchemy to see how things are arranged there, and what it
supports. SQLalchemy is feature-rich, but leaves out more
expressive-style ranges, and only supports unpreceding following/current
row, unbound following. I guess this is a reasonable limitation to make.

5. Will a 2.0 release notes be introduced soon, as I'm certain I won't
manage to make the final PR before the 1.11 feature freeze as it needs
to be finished, reviewed and merged. I started some time ago, and had it
lying around till I understood window functions better.

As pointed out in #26608, I have some rough, drafty code that's quite
far from a PR. I'm more interested in input for the above.

I'm relatively new to Django internals (it's fun and educational hacking
on them), and it took me some time to read through the code and
documentation for the ORM, and there are many facets of it I can still
understand. If someone is willing to guide me a bit, I'd be quite
helpful. Thank you.
-- 
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/482d2667-dea7-fd17-e820-6d6a0be08ae4%40inducks.org.
For more options, visit https://groups.google.com/d/optout.

MariaDB, official support

[Mads Jensen]

Hi,

I got interest in contributing to Django. I started working on a bigger
ticket than the smaller contributions I've previously made.

Adam Chainz pointed out that MariaDB will include support for window
expressions (#26608) from 10.2. I asked on #django-dev what the official
policy for MariaDB is, and was told it's ad-hoc. Also, the test
environment does include MariaDB which to me would make sense to add. I
think the implemented features for MySQL is also available for MariaDB.

I know the existence of django-mysql.
-- 
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/08a78eb8-c781-e2a0-240b-439f2717c87a%40inducks.org.
For more options, visit https://groups.google.com/d/optout.