Opened 11 years ago

Closed 7 days ago

#25243 closed Bug (fixed)

inspectdb crashes if SQLite foreign key references sqlite_master

Reported by: Stephan Sokolow Owned by: Sidharth Dusanapudi
Component: Core (Management commands) Version: 1.8
Severity: Normal Keywords:
Cc: dina.paolo@…, Sidharth Dusanapudi Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

While trying to import an SQLite database from an old legacy codebase, I ran up against the following exception in Django 1.8.3:

ssokolow@monolith XXXXXXX [django] % python manage.py inspectdb >| models.py
Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/core/management/base.py", line 393, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/core/management/base.py", line 444, in execute
    output = self.handle(*args, **options)
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/core/management/commands/inspectdb.py", line 25, in handle
    for line in self.handle_inspection(options):
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/core/management/commands/inspectdb.py", line 64, in handle_inspection
    relations = connection.introspection.get_relations(cursor, table_name)
  File "/home/ssokolow/.virtualenvs/XXXXXXX/local/lib/python2.7/site-packages/django/db/backends/sqlite3/introspection.py", line 128, in get_relations
    result = cursor.fetchall()[0]
IndexError: list index out of range

Examining the output revealed that it was dying in a table named "todos" and, examing that further, I discovered that inspectdb did not like what the code was doing to partially enforce a home-built generic foreign key constraint.

Here's the schema which caused it to fail:

CREATE TABLE todos (
    id INTEGER PRIMARY KEY,
    row_id INTEGER,
    table_name VARCHAR(64) DEFAULT 'stories' REFERENCES sqlite_master (tbl_name) ON DELETE RESTRICT ON UPDATE CASCADE COLLATE NOCASE,
    content TEXT NOT NULL CHECK(TRIM(content) <> '' AND TRIM(content) = content AND content NOT LIKE '%  %' AND content NOT GLOB '*[    

]*')
);

I had to manually dump the database to SQL, edit out this clause in vim (because SQLite's ALTER TABLE is so limited), and then re-create the database before inspectdb would successfully complete:

REFERENCES sqlite_master (tbl_name) ON DELETE RESTRICT ON UPDATE CASCADE

At the very least, it should probably have a clearer error message.

Change History (11)

comment:1 by Stephan Sokolow, 11 years ago

Oh, please note that I'll have to manually check this for updates as this Trac installation requires some kind of authentication to access RSS feeds.

(After many bad experiences with leaks to spammers, I've learned to not trust Trac installations with my e-mail aliases.)

comment:2 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Paolo Dina, 10 years ago

Cc: dina.paolo@… added

comment:4 by Claude Paroz, 8 years ago

I'm just posting a temporary test case diff (very WIP):

  • tests/backends/sqlite/test_introspection.py

    diff --git a/tests/backends/sqlite/test_introspection.py b/tests/backends/sqlite/test_introspection.py
    index 1695ee549e..6a4508031d 100644
    a b class IntrospectionTests(TestCase):  
    2525                        self.assertEqual(field, expected_string)
    2626                    finally:
    2727                        cursor.execute('DROP TABLE test_primary')
     28
     29    def test_references_sqlite_master(self):
     30        with connection.cursor() as cursor:
     31            sql = (
     32                'CREATE TABLE todos ('
     33                '    id INTEGER PRIMARY KEY,'
     34                '    pg INTEGER REFERENCES sqlite_master (root_page)'
     35                ')'
     36            )
     37            res = cursor.execute(sql)
     38            try:
     39                relations = connection.introspection.get_relations(cursor, 'todos')
     40                # What should we expect here?
     41                self.assertEqual(relations, {})
     42            finally:
     43                # I cannot understand why the following DROP fails (due to the FK somewhere).
     44                cursor.execute('DROP TABLE todos')

I'm not sure if it's possible to find the relation in any way...

comment:5 by Sidharth Dusanapudi, 3 months ago

Has patch: set

PR available: https://github.com/django/django/pull/20928

Adds regression test coverage for this ticket and sets Has patch for review.

comment:6 by Sidharth Dusanapudi, 3 months ago

Cc: Sidharth Dusanapudi added
Owner: changed from nobody to Sidharth Dusanapudi
Status: newassigned

comment:7 by ontowhee, 9 days ago

I tried to reproduce the bug using the schema provided in the description, but did not encounter any errors. It seems changes in commit 483e30c3d5 also resolved this issue. Would we want to close this ticket as "works for me", or proceed with adding the regressions test that Sidharth provided?

comment:8 by Jacob Walls, 8 days ago

Patch needs improvement: set

I think we generally add meaningful regression tests when we find accidentally fixed tickets, so +1 for keeping the PR. Thanks for confirming the fix. Left a note about a rename.

comment:9 by Sidharth Dusanapudi, 8 days ago

Updated the PR to use sqlite_schema instead of sqlite_master per the review feedback. Thanks!

Last edited 8 days ago by Sidharth Dusanapudi (previous) (diff)

comment:10 by Jacob Walls, 7 days ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:11 by GitHub <noreply@…>, 7 days ago

Resolution: fixed
Status: assignedclosed

In bd1cd8f:

Fixed #25243 -- Added inspectdb regression test for sqlite_schema FK.

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