Code

Opened 3 years ago

Closed 3 years ago

#14733 closed (fixed)

Allow Manager.raw() execute not only "Pure selects"

Reported by: dzentoo Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Since the raw query validation only matches if the request startswith("select") I would like to execute queries like "(SELECT) UNION (SELECT)".

Attachments (2)

force_select_raw_queries.patch (1.4 KB) - added by dzentoo 3 years ago.
attached patch
14733-xof.diff (1.8 KB) - added by Xof 3 years ago.
Per discussion on django-developers, a patch to remove the restrictions and update the documentation

Download all attachments as: .zip

Change History (15)

Changed 3 years ago by dzentoo

attached patch

comment:1 Changed 3 years ago by russellm

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Like I said in IRC - the use cases for raw() are fairly strictly limited to SELECT clauses; I'd rather see the validation logic improved to catch the edge cases, rather than just dropping validation.

comment:2 Changed 3 years ago by dzentoo

would a regexp suffice ?

comment:3 Changed 3 years ago by russellm

Yeah - a regex should do the job.

comment:5 Changed 3 years ago by russellm

Right now? No. Could it? Sure. You just need to make the validation sufficiently flexible.

comment:6 Changed 3 years ago by kemar

Why are raw queries strictly limited to SELECT queries? Is this a security measure?
Seems like determining whether a query is a SELECT or not require a complex regex.

comment:7 Changed 3 years ago by russellm

It's not a security measure -- it's a matter of basic functionality. The rest of the behavior of raw() deals with converting the returned results into Django objects. The query validation is purely about ensuring that the query will actually return results that can be converted into Django objects.

comment:8 Changed 3 years ago by dzentoo

Well, I tried "sqlparse" without success ..

res = sqlparse.parse("(SELECT * FROM pouet LIMIT 5) UNION (SELECT * FROM repouet)")
st = res[0]
print st.get_type()
'UNKNOWN'

What If I match if the query does not contain UPDATE / INSERT / DELETE ?
what do you mean by 'flexible validation' ? Any idea on how you 'll proceed ?

comment:9 Changed 3 years ago by SmileyChris

  • Version changed from 1.2 to SVN

http://thebuild.com/blog/2010/12/13/very-large-result-sets-in-django-using-postgresql/ also has a use case for wanting to use FETCH 1000 FROM gigantic_cursor with raw().

Really, it feels like we're doing too much validation here - raw is supposed to be for the edge cases and we're artificially limiting how it can be used.

comment:10 Changed 3 years ago by Xof

I think that the validation is trying too hard to make this foolproof. It's not practical to integrate into Django a validation across all possible backends to catch all and only set-returning statements that can be passed in. That's not even in a static set of operations (for example, right now, the DO statement in PostgreSQL can't return sets, but there are proposals to allow it to do so).

comment:11 Changed 3 years ago by ramiro

#15209 was a duplicate.

comment:12 Changed 3 years ago by ramiro

See also #14898, #13678 was a duplicate.

Changed 3 years ago by Xof

Per discussion on django-developers, a patch to remove the restrictions and update the documentation

comment:13 Changed 3 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

In [15803]:

Fixed #14733: no longer "validate" .raw() queries.

Turns out that a lot more than just SELECT can return data, and this list is
very hard to define up front in a cross-database manner. So let's just assume
that anyone using raw() is at least halfway competant and can deal with
the error messages if they don't use a data-returning query.

Thanks to Christophe Pettus for the patch.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.