Opened 2 years ago

Closed 2 years ago

#19676 closed Bug (fixed)

Inspectdb ForeignKey for self

Reported by: Georgy Kutsurua Owned by: charettes
Component: Uncategorized Version: master
Severity: Normal Keywords: inspectdb foreignkey
Cc: suquant Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Inspectdb command generate ForeignKey string

parent = models.ForeignKey(''self'', null=True, blank=True)

it is uncorrectly, we needed

parent = models.ForeignKey('self', null=True, blank=True)

here we use one quote instead double quote :)

Attachments (2)

0001-Fixed-19676-Avoid-quoting-recursive-ForeignKey-targe.patch (2.9 KB) - added by charettes 2 years ago.
19676-2.patch (2.8 KB) - added by aaugustin 2 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 2 years ago by suquant

  • Cc suquant added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by charettes

  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.5-beta-1 to master

Managed to reproduce on postgresql (which supports db fks).

comment:3 Changed 2 years ago by charettes

  • Owner changed from nobody to charettes
  • Status changed from new to assigned

comment:4 Changed 2 years ago by charettes

I think I found another bug while debugging this, SQLite supposedly can_introspect_foreign_keys but fails with recursive ones.

Version 0, edited 2 years ago by charettes (next)

comment:5 Changed 2 years ago by charettes

  • Has patch set

comment:6 Changed 2 years ago by charettes

Attached a patch that passes on Python 2.7.3 and 3.2.3 SQLite. It requires the fix for #19677 to pass on SQLite.

comment:7 Changed 2 years ago by aaugustin

Although the patch looks good, the test fails on SQLite for me — the FK isn't detected and the output contains parent_id = models.IntegerField().

(django-dev)myk@mYk tests % PYTHONPATH=.. python runtests.py --settings=test_sqlite inspectdb
Creating test database for alias 'default'...
Creating test database for alias 'other'...
F...
======================================================================
FAIL: test_attribute_name_not_python_keyword (regressiontests.inspectdb.tests.InspectDBTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/myk/Documents/dev/django/django/test/testcases.py", line 873, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/myk/Documents/dev/django/tests/regressiontests/inspectdb/tests.py", line 35, in test_attribute_name_not_python_keyword
    self.assertIn("parent = models.ForeignKey('self')", output)
AssertionError: u"parent = models.ForeignKey('self')" not found in "# This is an auto-generated Django model module.\n# You'll have to do the following manually to clean this up:\n#     * Rearrange models' order\n#     * Make sure each model has one field with primary_key=True\n# Feel free to rename the models, but don't rename db_table values or field names.\n#\n# Also note: You'll have to insert the output of 'django-admin.py sqlcustom [appname]'\n# into your database.\nfrom __future__ import unicode_literals\n\nfrom django.db import models\n\nclass InspectdbDigitsincolumnname(models.Model):\n    id = models.IntegerField(primary_key=True)\n    number_123 = models.CharField(max_length=11, db_column='123') # Field renamed because it wasn't a valid Python identifier.\n    number_4extra = models.CharField(max_length=11, db_column='4extra') # Field renamed because it wasn't a valid Python identifier.\n    number_45extra = models.CharField(max_length=11, db_column='45extra') # Field renamed because it wasn't a valid Python identifier.\n    class Meta:\n        db_table = 'inspectdb_digitsincolumnname'\n\nclass InspectdbMessage(models.Model):\n    id = models.IntegerField(primary_key=True)\n    from_field = models.ForeignKey('InspectdbPeople', db_column='from_id') # Field renamed because it was a Python reserved word.\n    class Meta:\n        db_table = 'inspectdb_message'\n\nclass InspectdbPeople(models.Model):\n    id = models.IntegerField(primary_key=True)\n    name = models.CharField(max_length=255)\n    parent_id = models.IntegerField()\n    class Meta:\n        db_table = 'inspectdb_people'\n\nclass InspectdbPeopledata(models.Model):\n    people_pk = models.ForeignKey(InspectdbPeople, primary_key=True)\n    ssn = models.CharField(max_length=11)\n    class Meta:\n        db_table = 'inspectdb_peopledata'\n\nclass InspectdbPeoplemoredata(models.Model):\n    id = models.IntegerField(primary_key=True)\n    people_unique = models.ForeignKey(InspectdbPeople, unique=True)\n    license = models.CharField(max_length=255)\n    class Meta:\n        db_table = 'inspectdb_peoplemoredata'\n\nclass InspectdbSpecialcolumnname(models.Model):\n    id = models.IntegerField(primary_key=True)\n    field = models.IntegerField()\n    field_field = models.IntegerField(db_column='Field_') # Field name made lowercase. Field renamed because it ended with '_'.\n    field_field_0 = models.IntegerField(db_column='Field__') # Field name made lowercase. Field renamed because it contained more than one '_' in a row. Field renamed because it ended with '_'. Field renamed because of name conflict.\n    field_field_1 = models.IntegerField(db_column='__field') # Field renamed because it contained more than one '_' in a row. Field renamed because it started with '_'. Field renamed because of name conflict.\n    prc_x = models.IntegerField(db_column='prc(%) x') # Field renamed to remove unsuitable characters.\n    class Meta:\n        db_table = 'inspectdb_specialcolumnname'\n\n"

----------------------------------------------------------------------
Ran 4 tests in 0.021s

FAILED (failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

Changed 2 years ago by aaugustin

comment:8 Changed 2 years ago by aaugustin

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Patch needs improvement set

Patch updated with a small stylistic improvement.

comment:9 Changed 2 years ago by charettes

  • Component changed from Database layer (models, ORM) to Uncategorized
  • Patch needs improvement unset

It requires the fix for #19677 to pass on SQLite.

comment:10 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

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

In c47fa3b4814240bb47342a4446d40ea83bd3ed19:

Fixed #19676 -- Supported 'self' foreign keys in inspectdb.

Thanks Georgy Kutsurua for the report and Simon Charette for the patch.

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