Opened 7 years ago

Closed 7 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 Markus Holtermann, 7 years ago

Owner: changed from nobody to Markus Holtermann
Status: new → assigned

comment:2 by Claude Paroz, 7 years ago

Severity: Normal → Release blocker
Triage Stage: Unreviewed → Accepted

comment:3 by Markus Holtermann, 7 years ago

Has patch: set

PR: ​https://github.com/django/django/pull/7764

comment:4 by Tim Graham, 7 years ago

Triage Stage: Accepted → Ready for checkin

comment:5 by Markus Holtermann, 7 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:6 by Markus Holtermann, 7 years ago

And given that, I lean towards closing as wontfix or worksforme.

comment:7 by Markus Holtermann, 7 years ago

Resolution: → wontfix
Status: assigned → closed

Closing as per explanation above.

Note: See TracTickets for help on using tickets.
Back to Top