[Django] #24154: Fix check_aggregate_support for Expressions

14 views
Skip to first unread message

Django

unread,
Jan 15, 2015, 1:34:44 AM1/15/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: | Status: new
Uncategorized |
Component: Database | Version: 1.8alpha1
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The existing check_aggregate_support does not recursively check all sub-
expressions. Further, there is currently no way for non-aggregate
expressions to opt-in to backend checks.

Expressions should call a check_backend_support method (on the compiler or
connection) passing itself. It should also pass all sub-expressions
(get_source_expressions), so that we can recursively check for support.

Most expressions can be a no-op if they know for certain that all
backends will have support, like for the Col and Ref expressions. This
will improve performance by removing quite a bit of method call overhead.

The backend will then throw an exception if there is no support for a
particular combination.

--
Ticket URL: <https://code.djangoproject.com/ticket/24154>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 15, 2015, 1:48:30 AM1/15/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by akaariai):

* stage: Unreviewed => Accepted


Comment:

My current feeling is that we could perhaps do this in as_sql() for those
backends that need it. So, StdDev.as_sql() would read something like:
{{{
def as_sql(self, compiler, connection):
connection.check_support(self)
<original code>
}}}

While it is a bit ugly to check the support in as_sql(), this would allow
us to avoid any overhead for those expressions that do not need the check.
There is a justification for this, too. The as_sql() method should throw
an error if there is no way to generate SQL for the given connection.

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:1>

Django

unread,
Jan 17, 2015, 12:14:37 AM1/17/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: Uncategorized | Status: assigned

Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jarshwah):

* status: new => assigned
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin


Comment:

https://github.com/django/django/pull/3939

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:2>

Django

unread,
Jan 17, 2015, 4:09:50 AM1/17/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: | Status: assigned
Cleanup/optimization |

Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by claudep):

* type: Uncategorized => Cleanup/optimization


Comment:

I've just complemented a GIS test in
https://github.com/django/django/commit/d69ecf922ddf6d4e3698e0dfd42d9ae387df182c

If this still passes after your patch, I'm confirming the RFC status of
the patch.

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:3>

Django

unread,
Jan 20, 2015, 10:06:05 AM1/20/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:4>

Django

unread,
Jan 26, 2015, 10:42:58 PM1/26/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: | Status: closed

Cleanup/optimization |
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Josh Smeaton <josh.smeaton@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"8196e4bdf498acb05e6680c81f9d7bf700f4295c"]:
{{{
#!CommitTicketReference repository=""
revision="8196e4bdf498acb05e6680c81f9d7bf700f4295c"
Fixed #24154 -- Backends can now check support for expressions
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:5>

Django

unread,
Jan 26, 2015, 10:49:14 PM1/26/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Josh Smeaton <josh.smeaton@…>):

In [changeset:"e56810e839db2beddc8a7b6e917158855ef381dc"]:
{{{
#!CommitTicketReference repository=""
revision="e56810e839db2beddc8a7b6e917158855ef381dc"
[1.8.x] Fixed #24154 -- Backends can now check support for expressions

Backport of 8196e4bdf498acb05e6680c81f9d7bf700f4295c from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:6>

Django

unread,
Jan 26, 2015, 11:39:38 PM1/26/15
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: jarshwah
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Josh Smeaton <josh.smeaton@…>):

In [changeset:"e77c1bc181a27f8d591a94a2f462325e8722e565"]:
{{{
#!CommitTicketReference repository=""
revision="e77c1bc181a27f8d591a94a2f462325e8722e565"
Refs #24154 -- Added deprecation release notes
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:7>

Django

unread,
Dec 31, 2016, 2:06:20 PM12/31/16
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: Josh Smeaton | Owner: Josh
Type: | Smeaton
Cleanup/optimization | Status: closed

Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"2b4bb78f7944adbf88f9bf3b868632707c79b5dd" 2b4bb78f]:
{{{
#!CommitTicketReference repository=""
revision="2b4bb78f7944adbf88f9bf3b868632707c79b5dd"
Refs #24154 -- Added check_aggregate_support() to deprecation timeline.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:8>

Django

unread,
Dec 31, 2016, 2:06:38 PM12/31/16
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: Josh Smeaton | Owner: Josh
Type: | Smeaton
Cleanup/optimization | Status: closed
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"9e8bfc5c62a40826a136c02d91e276e129551e54" 9e8bfc5]:
{{{
#!CommitTicketReference repository=""
revision="9e8bfc5c62a40826a136c02d91e276e129551e54"
[1.9.x] Refs #24154 -- Added check_aggregate_support() to deprecation
timeline.

Backport of 2b4bb78f7944adbf88f9bf3b868632707c79b5dd from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:9>

Django

unread,
Dec 31, 2016, 2:06:46 PM12/31/16
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: Josh Smeaton | Owner: Josh
Type: | Smeaton
Cleanup/optimization | Status: closed
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"5ae186812c04ebe04f1205e72d76b8bcb509033e" 5ae1868]:
{{{
#!CommitTicketReference repository=""
revision="5ae186812c04ebe04f1205e72d76b8bcb509033e"
[1.10.x] Refs #24154 -- Added check_aggregate_support() to deprecation
timeline.

Backport of 2b4bb78f7944adbf88f9bf3b868632707c79b5dd from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:10>

Django

unread,
Jan 17, 2017, 10:09:51 PM1/17/17
to django-...@googlegroups.com
#24154: Fix check_aggregate_support for Expressions
-------------------------------------+-------------------------------------
Reporter: Josh Smeaton | Owner: Josh
Type: | Smeaton
Cleanup/optimization | Status: closed
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"a3bd8672d87466649e5f5b0fa27634d1800a29a7" a3bd867]:
{{{
#!CommitTicketReference repository=""
revision="a3bd8672d87466649e5f5b0fa27634d1800a29a7"
Refs #24154 -- Removed deprecated
BaseDatabaseOperations.check_aggregate_support().
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24154#comment:11>

Reply all
Reply to author
Forward
0 new messages