[Django] #25759: Add **kwargs to as_sql method of Expressions
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: nobody
Type: New | Status: new
feature |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords: expressions
Triage Stage: Accepted | Has patch: 0
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
A common pattern when overriding `as_sql` or implementing `as_vendor` with
expressions is to modify the template or function attributes before
rendering the expression. This isn't safe to do, because expressions can
be shared between backends.
It'd be a good idea to add `**kwargs` to the method signature, so that
overrides can add context to that specific method call without mutating
the expression itself. For expressions like `Func`, it could mix the
`kwargs` with `self.extra` during rendering. Unsure how `**kwargs` would
affect other expression types, but that can be opt in per type.
Once this is done, the documentation written for
https://github.com/django/django/pull/5574 for third party backend
overrides will need to be updated to take advantage of the new `kwargs`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25759>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: nobody
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: expressions | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Changes (by jarshwah):
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:1>
Django
-------------------------------------+-------------------------------------
| singh
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => shekhar-singh
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:2>
Django
-------------------------------------+-------------------------------------
| singh
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Comment (by jarshwah):
Some notes for completing this ticket:
Start by adding **kwargs to the method signature of as_sql for all
expression types, including transforms and lookups. There are quite a few
places.
For Func, get rid of the function and template key word arguments, and
merge self.extra with **kwargs in a local variable (so you don't modify
self.extra), using that to generate the sql string.
Need to update docs to reference the new **kwargs argument and change any
tests that override as_sql for vendor specific tests so that they no
longer modify self.template or self.function.
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Changes (by shekhar-singh):
* status: assigned => new
* owner: shekhar-singh =>
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* owner: => kaifeldhoff
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Changes (by kaifeldhoff):
* has_patch: 0 => 1
Comment:
Pull request: [https://github.com/django/django/pull/6147]
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by kaifeldhoff):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* easy: 1 => 0
Comment:
Comments for improvement are on the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Ready for checkin => Accepted
Comment:
New PR: https://github.com/django/django/pull/6302
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"baa8b0ec399bd9b0ff3308666b49c6ab734aa416" baa8b0ec]:
{{{
#!CommitTicketReference repository=""
revision="baa8b0ec399bd9b0ff3308666b49c6ab734aa416"
Refs #25759 -- Fixed some Funcs to work if different database backends are
used.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"5ca08f7cab4d65f9bd49bd8a1817dd4b6591cab1" 5ca08f7c]:
{{{
#!CommitTicketReference repository=""
revision="5ca08f7cab4d65f9bd49bd8a1817dd4b6591cab1"
Refs #25759 -- Documented customizing expressions' SQL on other databases.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/6338 Updated PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner:
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: expressions | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"53361589905c7c07692cd77f398ce6cb5ac39779" 53361589]:
{{{
#!CommitTicketReference repository=""
revision="53361589905c7c07692cd77f398ce6cb5ac39779"
Fixed #25759 -- Added keyword arguments to customize Expressions'
as_sql().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:16>
Django
-------------------------------------+-------------------------------------
| Feldhoff
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"d5977e492e6e8eb7ba0ac81f85c27d56db85b78c" d5977e49]:
{{{
#!CommitTicketReference repository=""
revision="d5977e492e6e8eb7ba0ac81f85c27d56db85b78c"
Refs #25759 -- Fixed some GIS Funcs if different database backends are
used.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25759#comment:17>