Opened 8 years ago
Closed 8 years ago
#27663 closed Cleanup/optimization (wontfix)
Make Index.create_sql() and SchemaEditor._create_index_sql() return SQL parameters as well
Reported by: | Markus Holtermann | Owned by: | Markus Holtermann |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Looking into #26167, I noticed that, in order to eventually have something like FunctionalIndex(Concat(Lower(Ref('some_field', None)), Value('x')))
, Django needs to return the params for the SQL query and pass them to the SchemaEditor's execute()
method.
When constructing the SQL for the expression Concat(Lower(Ref('some_field', None)), Value('x')).as_sql(...)
, Django returns the SQL with placeholders for the values:
>>> Concat(Lower(Ref('some_field', None)), Value('x')).as_sql(compiler, connection) ('CONCAT(LOWER("some_field"), %s)', ['x'])
Since the whole API for Index.create_sql()
and SchemaEditor.add_index()
etc. API is new in Django 1.11 let's try to get ahead of a necessary deprecation at a later time by passing the SQL statement and empty params by default, for now.
Change History (7)
comment:1 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 8 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 8 years ago
Has patch: | set |
---|
comment:4 by , 8 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:5 by , 8 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
Looking at #25809 again I noticed that SchemaEditor.quote_value()
is sufficient when creating the SQL query as none of the values put through that function are user defined but defined at development time by developers.
comment:7 by , 8 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Closing as per explanation above.
PR: https://github.com/django/django/pull/7764