Add **kwargs to as_sql method of Expressions
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
.
Change History
(17)
Owner: |
changed from nobody to Shekhar Singh Choudhary
|
Status: |
new → assigned
|
Owner: |
Shekhar Singh Choudhary removed
|
Status: |
assigned → new
|
Owner: |
set to Kai Feldhoff
|
Status: |
new → assigned
|
Needs documentation: |
unset
|
Needs tests: |
unset
|
Patch needs improvement: |
unset
|
Easy pickings: |
unset
|
Patch needs improvement: |
set
|
Patch needs improvement: |
unset
|
Triage Stage: |
Accepted → Ready for checkin
|
Triage Stage: |
Ready for checkin → Accepted
|
Patch needs improvement: |
set
|
Patch needs improvement: |
unset
|
Resolution: |
→ fixed
|
Status: |
assigned → closed
|
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.