What the purpose of having function that is not working correctly?

[Alexander Lyabah]

What the purpose of having function that is not working correctly, when you may not have this function at all and thing is changed.

I'm talking here about function Query.__str__

Bellow I show you an example:

In [19]: str(TimelineEvent.objects.filter(id__gt=100).query)

Out[19]: 'SELECT "timeline_timelineevent"."id", "timeline_timelineevent"."created_on", "timeline_timelineevent"."modified_on", "timeline_timelineevent"."action_on", "timeline_timelineevent"."timeline_id", "timeline_timelineevent"."content_type_id", "timeline_timelineevent"."object_id", "timeline_timelineevent"."action", "timeline_timelineevent"."meta" FROM "timeline_timelineevent" WHERE "timeline_timelineevent"."id" > 100 ORDER BY "timeline_timelineevent"."action_on" DESC, "timeline_timelineevent"."id" DESC'

​

In [20]: Timeline.objects.raw(str(TimelineEvent.objects.filter(id__gt=100).query))

Out[20]: <RawQuerySet: SELECT "timeline_timelineevent"."id", "timeline_timelineevent"."created_on", "timeline_timelineevent"."modified_on", "timeline_timelineevent"."action_on", "timeline_timelineevent"."timeline_id", "timeline_timelineevent"."content_type_id", "timeline_timelineevent"."object_id", "timeline_timelineevent"."action", "timeline_timelineevent"."meta" FROM "timeline_timelineevent" WHERE "timeline_timelineevent"."id" > 100 ORDER BY "timeline_timelineevent"."action_on" DESC, "timeline_timelineevent"."id" DESC>

​

In [21]: len(Timeline.objects.raw(str(TimelineEvent.objects.filter(id__gt=100).query)))

Out[21]: 89827

​

In [23]: str(TimelineEvent.objects.filter(id__gt=100, created_on__gt=datetime(2020, 1,1)).query)

Out[23]: 'SELECT "timeline_timelineevent"."id", "timeline_timelineevent"."created_on", "timeline_timelineevent"."modified_on", "timeline_timelineevent"."action_on", "timeline_timelineevent"."timeline_id", "timeline_timelineevent"."content_type_id", "timeline_timelineevent"."object_id", "timeline_timelineevent"."action", "timeline_timelineevent"."meta" FROM "timeline_timelineevent" WHERE ("timeline_timelineevent"."created_on" > 2020-01-01 00:00:00+00:00 AND "timeline_timelineevent"."id" > 100) ORDER BY "timeline_timelineevent"."action_on" DESC, "timeline_timelineevent"."id" DESC'

​

In [24]: len(Timeline.objects.raw(str(TimelineEvent.objects.filter(id__gt=100, created_on__gt=datetime(2020, 1, 1)).query)))

---------------------------------------------------------------------------

ProgrammingError Traceback (most recent call last)

/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py in _execute(self, sql, params, *ignored_wrapper_args)

83 else:

---> 84 return self.cursor.execute(sql, params)

85

​

ProgrammingError: syntax error at or near "00"

LINE 1: ...timeline_timelineevent"."created_on" > 2020-01-01 00:00:00+0...

^

And this function is recommended on stackoverflow all other the place.

What would change if instead of

def __str__(self):
        sql, params = self.sql_with_params()
        return sql % params

We will have

def __str__(self):
        return '{} | {}'.format(sql, params)

In that case nobody will ever want to use it as argument for raw function.

Thank you. And please, let me know what you think about it

[Adam Johnson]

What the purpose of having function that is not working correctly?

You'll go further in life if you use more descriptive, less inflammatory subjects.

Also it's polite to search the ticket tracker for related issues before posting on the list. This is the relevant ticket: https://code.djangoproject.com/ticket/25705

Query.__str__ has never been intended as more than a debugging aid. Part of the reason it's hard to compute the *actual* SQL is because Django doesn't do it - it's done by the database library on cursor.execute(). This is more than string quoting - some objects have special adapters, for example JSON values. On top of this, there's no standard method in the database libraries for SQL interpolation - it was never specified in DB API ( https://www.python.org/dev/peps/pep-0249/ ).

psycopg2 has a mogrify() function, but the other libarries don't - it may require calling some internal/undocumented methods. Solving this ticket needs some research and work. Your contributions would be welcome.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/44595903-f679-4d08-abb8-80d2c8dec6e9n%40googlegroups.com.

--

Adam

[Moses Mugisha]

+1

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM3vjyUKPwJrKvmA5Rfp9-bZGykw8e6QrpWRCoNsg1BWnA%40mail.gmail.com.

[Alexander Lyabah]

I'm sorry for not being polite. Believe my I love what you have built already and what to support in any way I can.

My question is exactly what is in the subject is, it is not some kind of clickbait. I've done some research before, and I understand the complexity of the task you want to accomplish in the linked ticket.

My question and concerns are more about - why we have it in the first place. If the intention is to debug rather than using in production - we can change the function in a way I suggested above, and no need to close the linked ticket at all.

The problem with the function is that it is actually working, but not always, and because of that, other people are suggesting it on StackOverflow, using it in prod, and may, eventually catch weird exceptions, which leads to a bad experience with Django in general.

Also, want to say it again, English is not my first language, and some words may sound not polite at all. It is not my intention, I respect all the work you have done with Django, and very thankful for continue working on it.

[Mariusz Felisiak]

We also have other related tickets #24803 and #24991. Please take a look at the last PR and feel-free to continue it, if you really want to fix these issues.

Best,
Mariusz

[אורי‎]

On Thu, Sep 10, 2020 at 12:17 PM Alexander Lyabah <a.ly...@checkio.org> wrote:

Also, want to say it again, English is not my first language, and some words may sound not polite at all. It is not my intention, I respect all the work you have done with Django, and very thankful for continue working on it.

+1

[Florian Apolloner]

On Thursday, September 10, 2020 at 11:16:56 AM UTC+2 Alexander Lyabah wrote:

The problem with the function is that it is actually working, but not always, and because of that, other people are suggesting it on StackOverflow, using it in prod, and may, eventually catch weird exceptions, which leads to a bad experience with Django in general.

No, this function is never working in a useful way. It does client side interpolation of query params which should be done by the drivers instead, even when it works it is potentially dangerous. The actual problem is that people on StackOverflow recommend to use it. FWIW Since it is solely a debugging aid I'd actually break any usage of it by adding "--" to the start of it (or similar)

 

[Alexander Lyabah]

I'm sorry. Now things sound even more confusing for me.

From one side you've said that "No, this function is never working in a useful way." (but my example from the post shows, how it works in the beginning and then stops working for datatime, which means it was pretty much useful for some very common cases)

From another side, you have accepted tickets, to make this function works in a useful way.

So it very looks like you've made a function, that returns something, that very looks like SQL, but shouldn't be used as SQL, it is just for debug, and you have a bunch of tickets to make SQL-like debug text to be working as real SQL.

So, my humble suggestion here is very simple. If you don't want something to be used in an appropriate way, don't make it looks like it can be used this way. 

[Fran Hrženjak]

Just my $.02, literally yesterday I saw a str(queryset.query) used to construct a raw SQL query. It of course suffers from the worst kind of SQL injection as well.

+1 to make it obvious, somehow, that kittens die every time it is used for a real query.

On 11 Sep 2020, at 15:58, Alexander Lyabah <a.ly...@checkio.org> wrote:

I'm sorry. Now things sound even more confusing for me.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/487cadce-51db-418f-aca4-ebe14aa16bb9n%40googlegroups.com.

[Tim Allen]

I've seen recommendations to use this during conference talks by people with a fairly deep knowledge of the ORM as recently as 2019, so I do believe it can be made more blatantly clear in its purpose. We took a stab at improving it during DjangoCon 2019. Consensus was that:

(a) when the underlying DB driver provides a method of constructing the query, a la PostgreSQL's `mogrify`, we can use it;

(b) when the underlying DB driver does NOT provide a method, we can make it obvious not to use it for more than debugging. IIRC, Tim Graham suggested we construct the string to be something like, """SQL: SELECT ... FROM ... WHERE x = ? AND y = ? PARAMS: ("this", "that")""" to provide the relevant information, but make it very obvious you can't copy pasta it into a SQL command line tool.

Here's the PR we were working on, which also contains some good discussion and background in addition to Claude's PR: https://github.com/django/django/pull/10568

...but ultimately, we decided Claude's approach linked by Mariusz above was better. I'd love to see this cross the finish line one of these days; my burning use case was eventually solved when `queryset.explain()` was introduced in Django 2.1.

[babloo chowhan]

I have an issue in a django project please can you help me.?

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/81ed71d5-f83e-4e36-8cb9-f718dd33515en%40googlegroups.com.