{"action":"created","issue":{"url":"https://api.github.com/repos/justquick/django-activity-stream/issues/238","labels_url":"https://api.github.com/repos/justquick/django-activity-stream/issues/238/labels{/name}","comments_url":"https://api.github.com/repos/justquick/django-activity-stream/issues/238/comments","events_url":"https://api.github.com/repos/justquick/django-activity-stream/issues/238/events","html_url":"https://github.com/justquick/django-activity-stream/pull/238","id":73445223,"number":238,"title":"Replace problamatic CharField GFKs with PositiveInterField fields.","user":{"login":"Shanto","id":821784,"avatar_url":"https://avatars.githubusercontent.com/u/821784?v=3","gravatar_id":"","url":"https://api.github.com/users/Shanto","html_url":"https://github.com/Shanto","followers_url":"https://api.github.com/users/Shanto/followers","following_url":"https://api.github.com/users/Shanto/following{/other_user}","gists_url":"https://api.github.com/users/Shanto/gists{/gist_id}","starred_url":"https://api.github.com/users/Shanto/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Shanto/subscriptions","organizations_url":"https://api.github.com/users/Shanto/orgs","repos_url":"https://api.github.com/users/Shanto/repos","events_url":"https://api.github.com/users/Shanto/events{/privacy}","received_events_url":"https://api.github.com/users/Shanto/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-05-05T22:06:59Z","updated_at":"2015-06-15T05:30:15Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/justquick/django-activity-stream/pulls/238","html_url":"https://github.com/justquick/django-activity-stream/pull/238","diff_url":"https://github.com/justquick/django-activity-stream/pull/238.diff","patch_url":"https://github.com/justquick/django-activity-stream/pull/238.patch"},"body":"Hi,\r\n\r\nI just noticed an error that when you try to use any Django feature that joins GFK enabled models with other models (e.g. having GenericRelation defined on that model with some related_query_name), it produces an SQL error (PostgreSQL).\r\n\r\nLet's take this example model:\r\n\r\n```python\r\nclass Post(models.Model):\r\n    content = models.TextField(blank=True, null=True)\r\n    position = models.PositiveSmallIntegerField(default=0, blank=True, editable=False)\r\n    action_set = GenericRelation('actstream.Action', object_id_field='action_object_object_id', content_type_field='action_object_content_type', related_query_name='post')\r\n```\r\n\r\nWe are using https://docs.djangoproject.com/en/1.8/ref/contrib/contenttypes/#reverse-generic-relations here with a related_query_name defined to help sorting/filtering activity streams based on this generic related model (Post) attributes. At this point, a query like below would produce some SQL error:\r\n\r\n```python\r\ntarget_stream(another_object).order_by('post__position')\r\n```\r\n\r\nThe error produced, in particular by PostgreSQL backend is:\r\n```\r\noperator does not exist: character varying = integer\r\n```\r\n\r\nFurther investigation reveals that because you are using CharField for GenericForeignKey's PK/ID part, and because by convention other Django models use PositiveIntegerField for their primary keys, Django's SQL compiler is NOT type casting the JOIN clause. Thus, the JOIN is failing from SQL backend due to an attempt to JOIN by (varchar == int) clause.\r\n\r\nI have successfully mitigated this issue by replacing your CharField definitions with PositiveIntegerField definitions. So, please have a look at my commit and let's discuss if there's any compelling reason to stick to CharFields.\r\n\r\nThanks,\r\nShanto"},"comment":{"url":"https://api.github.com/repos/justquick/django-activity-stream/issues/comments/111929344","html_url":"https://github.com/justquick/django-activity-stream/pull/238#issuecomment-111929344","issue_url":"https://api.github.com/repos/justquick/django-activity-stream/issues/238","id":111929344,"user":{"login":"justquick","id":35289,"avatar_url":"https://avatars.githubusercontent.com/u/35289?v=3","gravatar_id":"","url":"https://api.github.com/users/justquick","html_url":"https://github.com/justquick","followers_url":"https://api.github.com/users/justquick/followers","following_url":"https://api.github.com/users/justquick/following{/other_user}","gists_url":"https://api.github.com/users/justquick/gists{/gist_id}","starred_url":"https://api.github.com/users/justquick/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/justquick/subscriptions","organizations_url":"https://api.github.com/users/justquick/orgs","repos_url":"https://api.github.com/users/justquick/repos","events_url":"https://api.github.com/users/justquick/events{/privacy}","received_events_url":"https://api.github.com/users/justquick/received_events","type":"User","site_admin":false},"created_at":"2015-06-15T05:30:15Z","updated_at":"2015-06-15T05:30:15Z","body":"this was to fix #40 and have the maximum possible compatibility with other apps that could use any of int/char/uuid fields. i tried to make it 80% compatible with existing apps but you have hit the missing %20 with the post__position ordering clause. i bet there's a way to fix this using raw sql but since it's such a specific use case i dont see how it could be made generic enough to be included here. thoughts?"}}