Opened 4 years ago
Closed 4 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 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 4 years ago
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 by , 4 years ago
Simply wrapping table_name
in connection.ops.quote_name
should address the issue for anyone interested in picking the issue up.
comment:4 by , 4 years ago
Owner: | changed from | to
---|
a little guidance needed as this is my first ticket.
comment:5 by , 4 years ago
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:7 by , 4 years ago
"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 by , 4 years ago
shall i wrap "table_name" in "quote_name" as in "quote_name(table_name)"?
comment:9 by , 4 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:10 by , 4 years ago
shall i wrap "table_name" in "quote_name" as in "quote_name(table_name)"?
yes, self.ops.quote_name(table_name)
comment:11 by , 4 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:12 by , 4 years ago
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 by , 4 years ago
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 by , 4 years ago
Has patch: | set |
---|
comment:15 by , 4 years ago
Patch needs improvement: | set |
---|
Suggested test improvements to avoid the creation of another model.
comment:16 by , 4 years ago
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 by , 4 years ago
Since this issue hasn't received any activity recently, may I assign it to myself?
comment:19 by , 4 years ago
Owner: | changed from | to
---|
comment:21 by , 4 years ago
Patch needs improvement: | unset |
---|
comment:22 by , 4 years ago
Patch needs improvement: | set |
---|
comment:23 by , 4 years ago
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.