Opened 3 years ago
Closed 3 years ago
#32158 closed Bug (fixed)
loaddata crashes on SQLite when table names are SQL keywords.
Reported by: | Scott Herriman | Owned by: | Chinmoy |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.1 |
Severity: | Normal | Keywords: | loaddata SqlLite3 check_constraints |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
Steps to reproduce:
- Create a Model called
Order
. (order
is a SQL reserved word) - Create fixtures for the model
- Use manage.py loaddata to load the fixture.
- Notice that it fails with the following error. This is because the table name
order
is not quoted properly
(0.000) PRAGMA foreign_key_check(order); args=None Traceback (most recent call last): File "python3.7/site-packages/django/db/backends/utils.py", line 82, in _execute return self.cursor.execute(sql) File "python3.7/site-packages/django/db/backends/sqlite3/base.py", line 411, in execute return Database.Cursor.execute(self, query) sqlite3.OperationalError: near "order": syntax error
Root Cause
- File:
python3.7/site-packages/django/db/backends/sqlite3/base.py
line 327 - Function:
check_constraints
- Details: due to missing back ticks around
%s
in the SQL statementPRAGMA foreign_key_check(%s)
Here in check_constraints line 327 in context
if table_names is None: violations = cursor.execute('PRAGMA foreign_key_check').fetchall() else: violations = chain.from_iterable( cursor.execute('PRAGMA foreign_key_check(%s)' % table_name).fetchall() for table_name in table_names )
And here line 333
for table_name, rowid, referenced_table_name, foreign_key_index in violations: foreign_key = cursor.execute( 'PRAGMA foreign_key_list(%s)' % table_name ).fetchall()[foreign_key_index]
Issue confirmed in
- 3.1.0
- 3.1.2
Change History (24)
comment:1 Changed 3 years ago by
Owner: | changed from nobody to Nishant Sagar |
---|---|
Status: | new → assigned |
comment:2 Changed 3 years ago by
Summary: | SQLite3 DatabaseWrapper.check_constraints does not properly quote table names for PRAGMA statements causing a SQL statement failure → loaddata crashes on SQLite when table names are SQL keywords. |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 Changed 3 years ago by
Simply wrapping table_name
in connection.ops.quote_name
should address the issue for anyone interested in picking the issue up.
comment:4 Changed 3 years ago by
Owner: | changed from Nishant Sagar to Nayan sharma |
---|
a little guidance needed as this is my first ticket.
comment:5 Changed 3 years ago by
will the issue be fixed if i just wrap %s around back ticks -> %s
as in 'PRAGMA foreign_key_check(%s
)' and 'PRAGMA foreign_key_list(%s
)' % table_name?
comment:6 Changed 3 years ago by
Nayan, Have you seen Simon's comment? We should wrap with quote_name()
.
comment:7 Changed 3 years ago by
"Details: due to missing back ticks around %s in the SQL statement PRAGMA foreign_key_check(%s)"
But it is quoted that this should fix the issue.
comment:8 Changed 3 years ago by
shall i wrap "table_name" in "quote_name" as in "quote_name(table_name)"?
comment:9 Changed 3 years ago by
Owner: | Nayan sharma deleted |
---|---|
Status: | assigned → new |
comment:10 Changed 3 years ago by
shall i wrap "table_name" in "quote_name" as in "quote_name(table_name)"?
yes, self.ops.quote_name(table_name)
comment:11 Changed 3 years ago by
Owner: | set to George Bezerra |
---|---|
Status: | new → assigned |
comment:12 Changed 3 years ago by
First contribution, currently trying to understand the code to figure out how to write a regression test for this.
Any help in how to unit test this is appreciated.
comment:13 Changed 3 years ago by
I believe I got it. Will put my test in tests/fixtures_regress
which has a lot of regression tests for loaddata
already, creating a new Order
model and a fixture for it.
comment:14 Changed 3 years ago by
Has patch: | set |
---|
comment:15 Changed 3 years ago by
Patch needs improvement: | set |
---|
Suggested test improvements to avoid the creation of another model.
comment:16 Changed 3 years ago by
Things have been busy for me but I'm still on it, I'll do the changes to the patch later this week.
comment:17 Changed 3 years ago by
Since this issue hasn't received any activity recently, may I assign it to myself?
comment:19 Changed 3 years ago by
Owner: | changed from George Bezerra to Chinmoy |
---|
comment:20 Changed 3 years ago by
Patch awaiting review. PR: https://github.com/django/django/pull/13807
comment:21 Changed 3 years ago by
Patch needs improvement: | unset |
---|
comment:22 Changed 3 years ago by
Patch needs improvement: | set |
---|
comment:23 Changed 3 years ago by
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Thanks for the report, I was able to reproduce this issue with
db_table = 'order'
.Reproduced at 966b5b49b6521483f1c90b4499c4c80e80136de3.