Comment object_pk isn't correctly cast to text for lookups

[Florian Apolloner]

The title says it all... (​http://code.djangoproject.com/browser/django/trunk/django/contrib/comments/models.py#L22) Postgres throws a nice error:

   Caught an exception while rendering: operator does not exist: text = integer
LINE 1: ...public" = true  AND "django_comments"."object_pk" = 139  AND...
                                                             ^
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.

[Alex Gaynor]

This was originally done to allow the generic foreign key to point to a model with a cusotm non integer fkey, marking as DDN.

[Florian Apolloner]

If that's so we still need to fix it for Postgres.

[Jacob]

The problem isn't the use of the textfield; it's that the value isn't getting cast correctly -- it's probably a bug in GenericForeignKey.

[Jacob]

apollo13: can you post the entire traceback?

[Florian Apolloner]

there you go: ​http://dpaste.com/73906/. I am still not convinced of using TextFields here, I think a PositiveIntegerField is something every comment model writer could live with...

[Jacob]

... until they want to attach a comment to an object that doesn't use an integer as its primary key.

[Peter Baumgartner]

#8565 was closed as duplicate, but just so it is noted here, the default everywhere else in Django is to use object_id for the generic relation field name. It would make sense that comments used that instead of object_pk

[cmutel@…]

Explicitly cast id to string

[Chris Mutel]

My attachment just explicitly performs the correct casting. I am no Python expert, but this works for me (on posgresql). I think this is only a problem with new versions of psycopg, which are quite strict about types.

[Malcolm Tredinnick]

That fix looks like it should be roughly correct, although it should use smart_unicode() or smart_str(). It will currently break with non-ASCII data.

The problem, btw, is actually only caused by PostgreSQL 8.3, which has stopped auto-casting types. The error is coming from the database layer.

[cmutel@…]

Uses smart_unicode() instead of str()

[Chris Mutel]

Added revised patch which uses smart_unicode()

Read more about how 8.3 has changed, and what one can do to get previous behaviour, here:

​http://people.planetpostgresql.org/peter/index.php?/archives/18-Readding-implicit-casts-in-PostgreSQL-8.3.html

[anonymous]

Hmh, Postgresql 8.3 is almost getting to be more trouble than it is worth... I'm also seeing this.

[Jacob]

Fixed in [8631]

[anonymous]

That is [8641]

[martync]

The same problem seems to occur on generic.GenericRelation

class Article(models.Model):

comments = generic.GenericRelation(Comment, object_id_field="object_pk")

[Karen Tracey]

The commit message (though it cited the wrong ticket) stated the fix only applied to the specific problem noted here:

Fixed #8544: correctly cast Comment.object_pk to string when doing lookups. This really only papers over a bigger problem related to casting the RHS of GFKs, but that larger change can wait for a more systematic fix.

This problem is fixed. The larger problem needs its own ticket (if it doesn't have one already).

[Jacob]

Milestone 1.0 deleted