#8554 closed (fixed)
Comment object_pk isn't correctly cast to text for lookups
Reported by: | Florian Apolloner | Owned by: | nobody |
---|---|---|---|
Component: | contrib.comments | Version: | dev |
Severity: | Keywords: | ||
Cc: | Jannis Leidel, hanne.moa@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Attachments (3)
Change History (21)
by , 16 years ago
Attachment: | newcomments_fix.diff added |
---|
comment:1 by , 16 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:3 by , 16 years ago
Summary: | Newcomments should use PositiveIntegerField instead of TextField for object_pk → Comment object_pk isn't correctly cast to text for lookups |
---|
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
.
comment:4 by , 16 years ago
Component: | Uncategorized → django.contrib.comments |
---|---|
milestone: | → 1.0 |
Triage Stage: | Design decision needed → Accepted |
comment:6 by , 16 years ago
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...
comment:7 by , 16 years ago
... until they want to attach a comment to an object that doesn't use an integer as its primary key.
comment:8 by , 16 years ago
#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
comment:9 by , 16 years ago
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.
comment:10 by , 16 years ago
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.
comment:11 by , 16 years ago
Cc: | added |
---|
comment:12 by , 16 years ago
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
comment:13 by , 16 years ago
Cc: | added |
---|
Hmh, Postgresql 8.3 is almost getting to be more trouble than it is worth... I'm also seeing this.
comment:16 by , 15 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
The same problem seems to occur on generic.GenericRelation
class Article(models.Model):
comments = generic.GenericRelation(Comment, object_id_field="object_pk")
comment:17 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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).
This was originally done to allow the generic foreign key to point to a model with a cusotm non integer fkey, marking as DDN.