Code

Opened 4 years ago

Closed 9 months ago

#12728 closed Bug (fixed)

loadata/flush issues given GenericRelation, model inheritance and postgres

Reported by: pragmar Owned by: nobody
Component: contrib.contenttypes Version: master
Severity: Normal Keywords: psycopg2.ProgrammingError, GenericRelation
Cc: magma.chambers@…, joel@…, lorin@…, timograham@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

This issue was encountered while porting from mysql to postgres and resembles #11107, but under a entirely different set of circumstances. There appears to be something going wrong in the database operations with the combination of model inheritance and generic relations. I've boiled it down to the following example using postgres 8.4.2 and psycopg2.

models.py

from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

class ExampleBase(models.Model):        
    title = models.CharField('title', max_length=200, blank=True, null=True)    
    created = models.DateTimeField('created', auto_now_add=True)
    modified = models.DateTimeField('modified', auto_now=True)        
    def __unicode__(self):
        return u'%s' % self.title    

class Event(ExampleBase):    
    start = models.DateTimeField('start time')
    end = models.DateTimeField('end')    
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()    
    content_object = generic.GenericForeignKey("content_type", "object_id")
    
class Location(ExampleBase):
    address = models.CharField('address', max_length=200)
    city = models.CharField('city', max_length=50)
    state = models.CharField('state', max_length=50)    
    events = generic.GenericRelation(Event)

Given those models the error can be created by (1) running manage.py syncdb and (2) then running manage.py flush:

W:\development\web\example.com\example>manage.py flush
    Type 'yes' to continue, or 'no' to cancel: yes
Error: Database example couldn't be flushed. Possible reasons:
  * The database isn't running or isn't configured correctly.
  * At least one of the expected database tables doesn't exist.
  * The SQL was invalid.
Hint: Look at the output of 'django-admin.py sqlflush'. That's the SQL this command wasn't able to run.
The full error: relation "exampleapp_event_id_seq" does not exist
LINE 1: SELECT setval('"exampleapp_event_id_seq"', 1, false);

An identical error can be produced by via loaddata, here is a simple fixture for the above models, no generic relation objects need be created to trigger the error so long as the relationship is defined in the models:

[
  {
    "pk": 1, 
    "model": "exampleapp.examplebase", 
    "fields": {
      "created": "2010-01-29 08:27:10", 
      "modified": "2010-01-29 08:27:10", 
      "title": "test"
    }
  }, 
  {
    "pk": 1, 
    "model": "exampleapp.location", 
    "fields": {
      "city": "Boston", 
      "state": "MA", 
      "address": "1 Main St."
    }
  }
]
W:\development\web\example.com\example>manage.py loaddata test_fixture.json
Installing json fixture 'test_fixture' from absolute path.
Traceback (most recent call last):
  File "W:\development\web\example.com\example\manage.py", line 21, in <module>
    execute_manager(settings)
  File "C:\Python26\lib\site-packages\django\core\management\__init__.py", line 438, in execute_manager
    utility.execute()
  File "C:\Python26\lib\site-packages\django\core\management\__init__.py", line 379, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "C:\Python26\lib\site-packages\django\core\management\base.py", line 195, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "C:\Python26\lib\site-packages\django\core\management\base.py", line 222, in execute
    output = self.handle(*args, **options)
  File "C:\Python26\lib\site-packages\django\core\management\commands\loaddata.py", line 212, in handle
    cursor.execute(line)
psycopg2.ProgrammingError: column "id" does not exist
LINE 1: ... setval('"exampleapp_event_id_seq"', coalesce(max("id"), 1),...

I threw a print statement on the output of db.backends.postgresql.operations to see the full line (below):

['SELECT setval(\'"exampleapp_examplebase_id_seq"\', coalesce(max("id"), 1), max("id") IS NOT null) FROM "exampleapp_examplebase";',
 'SELECT setval(\'"exampleapp_event_id_seq"\', coalesce(max("id"), 1), max("id") IS NOT null) FROM "exampleapp_event";']

Let me know if there's more information I can provide. The same model hierarchy was working under mysql, and if I change the genericrelation to inherit from models.Model it works - so the issue requires a specific set of circumstances.

Thanks.

Attachments (2)

test_fixture.json (373 bytes) - added by pragmar 4 years ago.
unenlightened_workaround.diff (810 bytes) - added by pragmar 4 years ago.
workaround to achieve successful loaddata given GenericRelation and abstract class.

Download all attachments as: .zip

Change History (23)

Changed 4 years ago by pragmar

comment:1 Changed 4 years ago by pragmar

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

So I don't want to pretend I have a solution, obviously sniffing for a class name feels very wrong - but I throw this out there as a possible assist to someone more familiar than myself with the database backend - just trying to give due diligence to the ticket and point to where I think it's failing. I'm able to work around the issue with a few tweaks. Given current trunk (r12344):

django/db/backends/postgresql/operations.py (line 125)
if not f.rel.through:

  • becomes -

if not f.rel.through and not f.__class__.__name__ == 'GenericRelation':

additionally to fix flush, if I manually add the postgres sequence, it seems to work - perhaps the above modification omitted its creation?

Changed 4 years ago by pragmar

workaround to achieve successful loaddata given GenericRelation and abstract class.

comment:2 Changed 4 years ago by anonymous

  • Summary changed from loadata/flush issues using GenericRelation, abstract base class and postgres database to loadata/flush issues given GenericRelation, model inheritance and postgres

comment:3 Changed 4 years ago by russellm

  • milestone set to 1.2
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 4 years ago by russellm

  • milestone changed from 1.2 to 1.3

Not critical for 1.2

comment:5 Changed 3 years ago by mattmcc

  • milestone 1.3 deleted
  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 3 years ago by anonymous

  • Component changed from Database layer (models, ORM) to contrib.contenttypes

comment:7 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 2 years ago by chrischambers

  • Cc magma.chambers@… added

comment:10 Changed 2 years ago by ukch

  • Cc joel@… added

comment:11 Changed 21 months ago by lorinh

Note: This bug causes an issue in Mezzanine, a Django-based CMS.

comment:12 Changed 21 months ago by lorinh

It looks like the code in question assumes that the table has a column called "id", the query is hard-coded with 'id' in it:

                if not f.rel.through:
                    output.append("%s setval(pg_get_serial_sequence('%s','%s'), coalesce(max(%s), 1), max(%s) %s null) %s %s;" % \
                        (style.SQL_KEYWORD('SELECT'),
                        style.SQL_TABLE(qn(f.m2m_db_table())),
                        style.SQL_FIELD('id'),
                        style.SQL_FIELD(qn('id')),
                        style.SQL_FIELD(qn('id')),
                        style.SQL_KEYWORD('IS NOT'),
                        style.SQL_KEYWORD('FROM'),
                        style.SQL_TABLE(qn(f.m2m_db_table()))))

However, if the model uses multi-table inheritance, the table won't have an 'id' field.

comment:13 Changed 21 months ago by lorinh

Submitted a pull request here: https://github.com/django/django/pull/238

comment:14 Changed 21 months ago by charettes

  • Has patch set
  • Needs tests set

Can you add a failing testcase in fixtures_regress.tests?

I'm also asking myself if this could be related to #7565 somehow. See fixtures_regress.tests.TestFixtures.test_pg_sequence_resetting_checks

comment:15 Changed 21 months ago by lorinh

  • Cc lorin@… added
  • Needs tests unset

I added a failing test case by modifying one of the existing test models so that the backends.SequenceResetTest will now fail unless the patch is applied.

comment:16 Changed 19 months ago by lorinh

My original pull request was against the wrong branch in the django repository. This new pull request is against the master branch: https://github.com/django/django/pull/408

comment:17 Changed 15 months ago by timo

  • Triage Stage changed from Accepted to Ready for checkin

Patch applies cleanly and tests with postgresql_psycopg2 backend pass.

comment:18 Changed 11 months ago by timo

  • Cc timograham@… added
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

I could be mistaken, but it looks like the regression test doesn't seem to fail without the patch applied anymore (the patch also needs to be updated for the move of tests/regressiontests/backends/models.py to tests/backends/models.py).

comment:19 Changed 11 months ago by timo

Confirmed the new test no longer fails as of 97774429aeb54df4c09895c07cd1b09e70201f7d (#19385). I didn't investigate this further to see if the underlying issue is fixed or if this is still an issue and just needs a new test.

comment:21 Changed 9 months ago by timo

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

Ok, I will mark it as fixed.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.