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 statement PRAGMA 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 Nishant Sagar, 3 years ago

Owner: changed from nobody to Nishant Sagar
Status: newassigned

comment:2 by Mariusz Felisiak, 3 years ago

Summary: SQLite3 DatabaseWrapper.check_constraints does not properly quote table names for PRAGMA statements causing a SQL statement failureloaddata crashes on SQLite when table names are SQL keywords.
Triage Stage: UnreviewedAccepted

Thanks for the report, I was able to reproduce this issue with db_table = 'order'.

Reproduced at 966b5b49b6521483f1c90b4499c4c80e80136de3.

comment:3 by Simon Charette, 3 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 Nayan sharma, 3 years ago

Owner: changed from Nishant Sagar to Nayan sharma

a little guidance needed as this is my first ticket.

comment:5 by Nayan sharma, 3 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?

Last edited 3 years ago by Nayan sharma (previous) (diff)

comment:6 by Mariusz Felisiak, 3 years ago

Nayan, Have you seen Simon's comment? We should wrap with quote_name().

comment:7 by Nayan sharma, 3 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.

Last edited 3 years ago by Nayan sharma (previous) (diff)

comment:8 by Nayan sharma, 3 years ago

shall i wrap "table_name" in "quote_name" as in "quote_name(table_name)"?

comment:9 by Nayan sharma, 3 years ago

Owner: Nayan sharma removed
Status: assignednew

comment:10 by Simon Charette, 3 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 George Bezerra, 3 years ago

Owner: set to George Bezerra
Status: newassigned

comment:12 by George Bezerra, 3 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 George Bezerra, 3 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 George Bezerra, 3 years ago

Has patch: set

comment:15 by Simon Charette, 3 years ago

Patch needs improvement: set

Suggested test improvements to avoid the creation of another model.

comment:16 by George Bezerra, 3 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 Chinmoy, 3 years ago

Since this issue hasn't received any activity recently, may I assign it to myself?

comment:18 by Mariusz Felisiak, 3 years ago

Sure, feel-free.

comment:19 by Chinmoy, 3 years ago

Owner: changed from George Bezerra to Chinmoy

comment:21 by Simon Charette, 3 years ago

Patch needs improvement: unset

comment:22 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:23 by Mariusz Felisiak, 3 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:24 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 270072c:

Fixed #32158 -- Fixed loaddata crash on SQLite when table/column names are SQL keywords.

Note: See TracTickets for help on using tickets.
Back to Top