Opened 9 years ago

Last modified 3 months ago

#25705 new Cleanup/optimization

Parameters are not adapted or quoted in Query.__str__

Reported by: Dmitry Dygalo Owned by:
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Petr Přikryl, Ülgen Sarıkavak Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Now it is just string interpolation of the SQL template with parameters and in most cases produces invalid queries for the following reasons:

  • No quoting
  • No adaptation. So, some python objects will be used as is, not like their SQL equivalents

Yes, there are situations, when output of Query.__str__ is equal to actual query. But for debugging reasons, it will be better to see real query here. Also it is logical and expected behavior of this method - to show actual query.

Attachments (1)

ticket25705.patch (4.8 KB ) - added by Dmitry Dygalo 9 years ago.
Initial draft

Download all attachments as: .zip

Change History (11)

comment:1 by Dmitry Dygalo, 9 years ago

Has patch: set
Needs tests: set
Owner: changed from nobody to Dmitry Dygalo
Patch needs improvement: set
Status: newassigned

by Dmitry Dygalo, 9 years ago

Attachment: ticket25705.patch added

Initial draft

comment:3 by Tim Graham, 9 years ago

This looks like a duplicate of #17741 and #25092 which are "wontfix", however, I'm not sure I see any harm in your proposal besides the question of whether or not it can be implemented on other database backends.

comment:4 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

in reply to:  description comment:5 by Bernd Wechner, 5 years ago

Replying to Dmitry Dygalo:

Now it is just string interpolation of the SQL template with parameters and in most cases produces invalid queries for the following reasons:

  • No quoting
  • No adaptation. So, some python objects will be used as is, not like their SQL equivalents

Yes, there are situations, when output of Query.__str__ is equal to actual query. But for debugging reasons, it will be better to see real query here. Also it is logical and expected behavior of this method - to show actual query.

Fascinating. I inadvertantly filed a duplicate of this it seems:

https://code.djangoproject.com/ticket/30132#ticket

and I would dearly love this fixed. In essence Query.__str__ should return exactly what is logged as per here:

https://docs.djangoproject.com/en/2.1/faq/models/#how-can-i-see-the-raw-sql-queries-django-is-running

so we can reliably see and extract teh SQL of a query in a production environment in which DEBUG is not enabled!

I would gladly fix this and PR it, if I had the skills and I am close to that, as I am coding and debugging python, but single stepping into Query.__str__ didn't find me a quick easy answer and I bailed for now. So if you have any tips as to where the code is that Query.__str__ uses to generate SQL and where the code is that logs SQL to connection.queries I canbegin to look at it consider how this might be fixed.

It seems to me an experience Django coder could fix this in minutes and that there should be a regression test for this kind of code:

qs=model.objects.somequeryset
sql=str(qs.query)
raw_qs=model.objects.raw(sql)

I have a test bed in which I confirmed this failure here:

https://github.com/bernd-wechner/DjangoTutorial/blob/238787c83ef8515aeeb405577980e71ff35664e8/Library/views.py

Let me know how I might help get this fixed sooner, given ti was reported 3 years ago and is still not fixed!

comment:6 by GitHub <noreply@…>, 5 years ago

In 845042b3:

Refs #25705 -- Fixed invalid SQL generated by SQLFuncMixin.as_sql() in custom_lookups tests.

Generated SQL was invalid because parameters are quoted by a driver.

comment:7 by Petr Přikryl, 3 years ago

Cc: Petr Přikryl added

comment:8 by Mariusz Felisiak, 2 years ago

IMO we should close all related tickets as duplicates:

  • #24803 was marked as a duplicate (empty strings in parameters),
  • #24991 was marked as a duplicate (range types in parameters).

comment:9 by Mariusz Felisiak, 2 years ago

Owner: Dmitry Dygalo removed
Status: assignednew

comment:10 by Ülgen Sarıkavak, 3 months ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top