Re: importing fixtures to postgres fails to set sequences correctly

39 views
Skip to first unread message

Russell Keith-Magee

unread,
Jul 15, 2010, 7:40:35 AM7/15/10
to django-d...@googlegroups.com
On Thu, Jul 15, 2010 at 6:09 PM, Ales Zoulek <ales....@gmail.com> wrote:
> Hi guys,
>
> there seems to be a problem with django postgres backend, when
> importing data from fixtures. Data are imported correctly, but the
> sequences for primary keys are set incorrecly on models that have
> generic.GenericRelation field. See example:
>
> As a demo, those are just two models with generic relation.
>
> models.py:
> -------------
> from django.db import models
> from django.contrib.contenttypes import generic
> from django.contrib.contenttypes.models import ContentType
>
>
> class Tag(models.Model):
>    name = models.CharField(max_length=30)
>    content_type = models.ForeignKey(ContentType)
>    object_id = models.PositiveIntegerField()
>    content_object = generic.GenericForeignKey('content_type', 'object_id')
>
>
> class Post(models.Model):
>    name = models.CharField(max_length=30)
>    text = models.TextField()
>    tags = generic.GenericRelation('blog.Tag')
>
>
> The loaddata management command calls at the end
>
> connections[DEFAULT_DB_ALIAS].ops.sequence_reset_sql command
>
>  to update sq1 sequences. Let's see the result:
>
> ./manage shell
> -------------------
> In [1]: from django.db import DEFAULT_DB_ALIAS, connections
> In [2]: from django.core.management.color import no_style
> In [3]: from proj.blog.models import Post
> In [4]: connections[DEFAULT_DB_ALIAS].ops.sequence_reset_sql(no_style(), [Post])
> Out[4]:
> ['SELECT setval(pg_get_serial_sequence(\'blog_post\',\'id\'),
> coalesce(max("id"), 1), max("id") IS NOT null) FROM "blog_post";',
>  'SELECT setval(pg_get_serial_sequence(\'blog_post\',\'id\'),
> coalesce(max("id"), 1), max("id") IS NOT null) FROM "blog_tag";']
>
>
> As you can see, the problem is in the last "SELECT". Postgres backend
> "thinks" that Post.tags is m2m relation with usual m2m sql table and
> tries to update it's pk sequence. The table is of course non existant
> and it resets Post.pk sequence to max(Tag.pk), this is obviously
> incorrect behaviour and results with potential DatabaseErrors
> duplicate key on blog_post table.
> Removing Post.tags and accessing related tags directly works as a
> workaround, but not very nice and comfotable one..
>
>
>
> Do you agree that I'll start a ticket on that?

I haven't physically run the same code, but what you describe
certainly sounds plausible. Generic relations are a common edge case
that gets forgotten. On top of that, sequence resets on Postgres have
changed recently (post Django 1.2-final), so it's entirely possible
that a bug has slipped in.

A ticket would be most appreciated.

> I was looking to the code to make a patch, but since the error is
> caused by "magic" m2m field in application in contrib, I hasitate to
> make postgres backend "aware" of any generic.GenericRelation fields.
> On the other hand, I'm not sure it's possible to edit just
> GenericRelation class to fix that.

You shouldn't need to make the PostgreSQL backend aware of the
existence of GenericRelation. Fields contain a 'serialize' option that
is used to determine if a relation should be serialized as part of a
fixture. Not deserializing GenericRelations is the primary reason for
the existence of this setting. I'd need to check to be certain, but I
think the reason that GenericRelations can't be serialized will turn
out to be exactly the same as the reason they have their sequences
reset.

If it turns out that this isn't the case, I have no problem adding
another flag that *does* identify the right use case (i.e., not
"is_generic_relation", but "requires_sequence_reset", or something
like it).

Of related interest would be to check if this issue existed previously
(i.e., before the recent PostgreSQL sequence changes); if the issue
didn't exist in Django 1.2 final, then checking the diff between the
old and new implementations may also shed some light on potential
solutions.

Yours,
Russ Magee %-)

Ales Zoulek

unread,
Jul 15, 2010, 8:01:40 AM7/15/10
to django-d...@googlegroups.com
Okay,

ticket created as #13941

I'll try to find out more and let you know in this thread.

Thanks,

A.
------------------------------------------------------
Ales Zoulek
+420 604 332 515
Jabber: ales....@gmail.com
------------------------------------------------------

> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
>

Ales Zoulek

unread,
Jul 15, 2010, 9:48:16 AM7/15/10
to django-d...@googlegroups.com
Okay, so after a little bit more investigation on that topic, I've found out:

1] It's changed since changeset 13328
(http://code.djangoproject.com/changeset/13328)

2] Previous versions DID reset the sequence thru GenericRelation
fields, but it did it on the right table so it worked. It just called
useless code. In our example, the second query would set blog_tag
sequence to max(blog_tag.id)

3] I've created a patch and a test, that keeps the older behaviour.
Please see the ticket.

4] We can consider find a way how to skip the second sql query altogether.

A.

------------------------------------------------------
Ales Zoulek
+420 604 332 515
Jabber: ales....@gmail.com
------------------------------------------------------

Justin Bronn

unread,
Jul 15, 2010, 5:05:56 PM7/15/10
to Django developers
> > there seems to be a problem with django postgres backend, when
> > importing data from fixtures. Data are imported correctly, but the
> > sequences for primary keys are set incorrecly on models that have
> > generic.GenericRelation field. See example:

I have similar code, and was griping in #django-dev yesterday about
the regression. I came up with an almost identical fix (using
`m2m_db_table`), so I've accepted the ticket.

> Of related interest would be to check if this issue existed previously
> (i.e., before the recent PostgreSQL sequence changes); if the issue
> didn't exist in Django 1.2 final, then checking the diff between the
> old and new implementations may also shed some light on potential
> solutions.

It's definitely a side-effect of r13328 -- I have production code that
works fine on 1.2.X, but crashes on trunk without the patch when
restoring fixtures. My only question is whether there was an original
reason that `db_table` was used or if it was simply a copy & paste
oversight from the previous loop.

-Justin

Ramiro Morales

unread,
Jul 15, 2010, 9:03:05 PM7/15/10
to django-d...@googlegroups.com

I think so. And now I remember actually we discussed about this on
#django-dev a couple of weeks ago but I forgot to open a ticket:

http://botland.oebfare.com/logger/django-dev/2010/6/23/1/#02:04-2311026

Regards,

--
Ramiro Morales  |  http://rmorales.net

Reply all
Reply to author
Forward
0 new messages