Opened 12 years ago

Closed 12 years ago

#19676 closed Bug (fixed)

Inspectdb ForeignKey for self

Reported by: Georgy Kutsurua Owned by: Simon Charette
Component: Uncategorized Version: dev
Severity: Normal Keywords: inspectdb foreignkey
Cc: Georgy Kutsurua 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 Simon Charette 12 years ago.
19676-2.patch (2.8 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 by Georgy Kutsurua, 12 years ago

Cc: Georgy Kutsurua added

comment:2 by Simon Charette, 12 years ago

Triage Stage: UnreviewedAccepted
Version: 1.5-beta-1master

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

comment:3 by Simon Charette, 12 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:4 by Simon Charette, 12 years ago

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

Version 0, edited 12 years ago by Simon Charette (next)

comment:5 by Simon Charette, 12 years ago

Has patch: set

comment:6 by Simon Charette, 12 years ago

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 by Aymeric Augustin, 12 years ago

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'...

by Aymeric Augustin, 12 years ago

Attachment: 19676-2.patch added

comment:8 by Aymeric Augustin, 12 years ago

Component: UncategorizedDatabase layer (models, ORM)
Patch needs improvement: set

Patch updated with a small stylistic improvement.

comment:9 by Simon Charette, 12 years ago

Component: Database layer (models, ORM)Uncategorized
Patch needs improvement: unset

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

comment:10 by Aymeric Augustin <aymeric.augustin@…>, 12 years ago

Resolution: fixed
Status: assignedclosed

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