--
Ticket URL: <https://code.djangoproject.com/ticket/16055>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* cc: victor.van.den.elzen@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:1>
* cc: joeri@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:2>
* stage: Unreviewed => Accepted
Comment:
Replying to [ticket:16055 anonymous]:
> When you have a generic foreign key with TextField / CharField
object_id, and try to use it in a filter with a model that has an integer
primary key, PostgreSQL 9.0.4 errors out with this:
> {{{
> DatabaseError: operator does not exist: integer = text
>
> HINT: No operator matches the given name and argument type(s). You
might need to add explicit type casts.
> }}}
>
> A small example:
I've verified this specific test case by modifying tests already present
in the test suite (added in [12353]). See the attached patch.
I see no failure with Postgres 8.3 (the version in which cast behavior
changes were introduced). Can you please test it with Postgres 9 and post
the results?
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:3>
* component: Database layer (models, ORM) => contrib.contenttypes
Comment:
I updated the tests to apply cleanly to master and verified the failure on
PostgreSQL 9.4 at 58c7ff39fb265754fb17ab8d7f8a1401b355777b (Django
1.10.dev).
{{{
======================================================================
ERROR: test_charlink_filter
(generic_relations_regress.tests.GenericRelationTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tim/code/django/tests/generic_relations_regress/tests.py",
line 57, in test_charlink_filter
list(OddRelation1.objects.filter(clinks__title='title'))
File "/home/tim/code/django/django/db/models/query.py", line 258, in
__iter__
self._fetch_all()
File "/home/tim/code/django/django/db/models/query.py", line 1074, in
_fetch_all
self._result_cache = list(self.iterator())
File "/home/tim/code/django/django/db/models/query.py", line 52, in
__iter__
results = compiler.execute_sql()
File "/home/tim/code/django/django/db/models/sql/compiler.py", line 839,
in execute_sql
cursor.execute(sql, params)
File "/home/tim/code/django/django/db/backends/utils.py", line 64, in
execute
return self.cursor.execute(sql, params)
File "/home/tim/code/django/django/db/utils.py", line 92, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/home/tim/code/django/django/db/backends/utils.py", line 64, in
execute
return self.cursor.execute(sql, params)
ProgrammingError: operator does not exist: integer = character varying
LINE 1: ...ON ("generic_relations_regress_oddrelation1"."id" = "generic...
^
HINT: No operator matches the given name and argument type(s). You might
need to add explicit type casts.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:5>
* Attachment "16055-test.diff" added.
Comment (by timgraham):
#20271 is a duplicate with an alternate test.
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:6>
* cc: mmitar@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:7>
* cc: brian@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:8>
* cc: Kye Russell (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:9>
* cc: Kevin Wiliarty (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:10>
Comment (by Simon Charette):
If anyone is interested in addressing this issue it should be doable by
adjusting `Join.as_sql` to add the appropriate cast when `lhs_col` and
`rhs_col`'s `.db_type(connection)` doesn't match.
Ideally most of the `Cast` logic would be moved to the database backend
operations so `Join.as_sql` can rely on it instead of having to depend on
`db.models` stuff.
A less invasive solution would be to make `GenericRelation`'s `join_field`
return an empty `get_joining_columns` and include the adapted condition in
`get_extra_restriction` instead but that seems like this should be solved
at the `Join` level instead since we don't offer a low level way of
completely configuring join conditions; we only allow to augment them
using `FilteredRelation`.
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:11>
Comment (by Nekmo):
I have made a hack for Django-guardian. In my case, this query is the one
that fails:
{{{
from customers.models import CustomerMemberObjectPermission
CustomerMemberObjectPermission.objects.filter(service_api__service__service_name='foo')
# Raise DatabaseError
}}}
My models:
{{{
class CustomerMemberObjectPermission(UserObjectPermission):
objects = CustomerMemberObjectPermissionManager()
class ServiceApi(Model):
customer_permissions = GenericRelation(CustomerMemberObjectPermission,
object_id_field='object_pk',
related_query_name='service_api')
}}}
This is the wrong sql:
{{{
SELECT "guardian_userobjectpermission"."id",
"guardian_userobjectpermission"."permission_id",
"guardian_userobjectpermission"."content_type_id",
"guardian_userobjectpermission"."object_pk",
"guardian_userobjectpermission"."user_id" FROM
"guardian_userobjectpermission" INNER JOIN "services_serviceapi" ON
("guardian_userobjectpermission"."object_pk" = "services_serviceapi"."id"
AND ("guardian_userobjectpermission"."content_type_id" = 22)) INNER JOIN
"services_service" ON ("services_serviceapi"."service_id" =
"services_service"."id") INNER JOIN "customers_customer" ON
("services_service"."customer_id" = "customers_customer"."id") WHERE
"customers_customer"."customer_code" = BCL
}}}
The error is in the inner join. Django-guardian uses a field of type
string for object_pk. But my id is a integer type:
{{{
"guardian_userobjectpermission"."object_pk" = "services_serviceapi"."id"
}}}
The solution is to use a cast:
{{{
CAST("guardian_userobjectpermission"."object_pk" AS integer) =
"services_serviceapi"."id"
}}}
Although I know it is not the best, my solution has been this hack thanks
to Simon Charette:
{{{
class CustomJoin(Join):
def as_sql(self, compiler, connection):
sql, params = super(CustomJoin, self).as_sql(compiler, connection)
sql = sql.replace('"guardian_userobjectpermission"."object_pk"',
'CAST("guardian_userobjectpermission"."object_pk" AS integer)')
return sql, params
class CustomSQLCompiler(SQLCompiler):
def compile(self, node, select_format=False):
if isinstance(node, Join):
node = CustomJoin(
node.table_name, node.parent_alias, node.table_alias,
node.join_type,
node.join_field, node.nullable, node.filtered_relation
)
return super(CustomSQLCompiler, self).compile(node, select_format)
class CustomQuery(Query):
def get_compiler(self, using=None, connection=None):
original_compiler = super(CustomQuery,
self).get_compiler(using=using, connection=connection)
return CustomSQLCompiler(original_compiler.query,
original_compiler.connection, original_compiler.using)
# return super(CustomQuery, self).get_compiler(using=using,
connection=connection)
class CustomerMemberObjectPermissionQuerySet(CustomerQuerySet):
def __init__(self, model=None, query=None, using=None, hints=None):
if not query:
query = CustomQuery(model)
super(CustomerMemberObjectPermissionQuerySet, self).__init__(
model=model, query=query, using=using, hints=hints
)
class CustomerMemberObjectPermissionManager(CustomerManager,
UserObjectPermissionManager):
def get_queryset(self):
return CustomerMemberObjectPermissionQuerySet(self.model,
using=self._db)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:12>
Comment (by Simon Charette):
Note that another elegant solution to this problem could be to add support
to transforms to `ForeignObject.to_fields` and `.from_fields` so you could
do something along these lines
{{{#!python
class Animal(models.Model):
name = models.TextField()
tags = generic.GenericRelation(Tag, from_field='id__text')
}}}
(assuming we'd register a `__text` lookup for `IntegerField`)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:13>
Comment (by Marc DEBUREAUX):
Manage to create something based on Simon Charette's suggestion.
Creating a custom `GenericRelation` with the following:
- `get_joining_columns` returning an empty tuple
- `get_extra_restriction` building the whole where clause with content
type **AND** primary key join with Cast
Here's the result:
{{{#!python
class CustomGenericRelation(GenericRelation):
def get_joining_columns(self, reverse_join=False):
joining_columns = super().get_joining_columns(reverse_join)
self.joining_columns = joining_columns
return tuple()
def get_extra_restriction(self, where_class, alias, remote_alias):
cond = super().get_extra_restriction(where_class, alias,
remote_alias)
from_field = self.model._meta.pk
to_field =
self.remote_field.model._meta.get_field(self.object_id_field_name)
lookup = from_field.get_lookup('exact')(
Cast(from_field.get_col(alias),
output_field=models.TextField()),
to_field.get_col(remote_alias))
cond.add(lookup, 'AND')
return cond
}}}
Giving the following:
- a model `common.History` having a `GenericForeignKey` with `object_id`
as text
- a model `generic.Twitter` having a `CustomGenericRelation` and a numeric
primary key
- and the queryset:
{{{#!python
q =
Twitter.objects.values('id').filter(id=1).annotate(count=Count('histories'))
print(q.query)
}}}
It gives the following SQL query:
{{{#!sql
SELECT
"generic_twitter"."id",
COUNT("common_history"."id") AS "count"
FROM "generic_twitter"
LEFT OUTER JOIN "common_history" ON
(((
"common_history"."content_type_id" = 29 AND
CAST("generic_twitter"."id" AS text) = "common_history"."object_id"
)))
WHERE "generic_twitter"."id" = 1
GROUP BY "generic_twitter"."id"
}}}
Instead of this one (normal behaviour):
{{{#!sql
SELECT
"generic_twitter"."id",
COUNT("common_history"."id") AS "count"
FROM "generic_twitter"
LEFT OUTER JOIN "common_history" ON
(
"generic_twitter"."id" = "common_history"."object_id" AND
("common_history"."content_type_id" = 29)
)
WHERE "generic_twitter"."id" = 1
GROUP BY "generic_twitter"."id"
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:14>
* cc: Marc DEBUREAUX (added)
* version: 1.3 => 3.2
Comment:
Change Django version number for visibility purposes.
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:15>
* cc: Sage Abdullah (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:16>
* cc: David Wobrock (added)
* owner: nobody => David Wobrock
* has_patch: 0 => 1
* status: new => assigned
Comment:
[https://github.com/django/django/pull/16632 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:17>
* needs_better_patch: 0 => 1
Comment:
Per Simon's comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:18>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:19>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:20>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:21>
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:22>
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:23>
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:24>
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:25>
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:26>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"9bbf97bcdb488bb11aebb5bd405549fbec6852cd" 9bbf97bc]:
{{{
#!CommitTicketReference repository=""
revision="9bbf97bcdb488bb11aebb5bd405549fbec6852cd"
Fixed #16055 -- Fixed crash when filtering against char/text
GenericRelation relation on PostgreSQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:27>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"8b1ff0da4b162e87edebd94e61f2cd153e9e159d" 8b1ff0da]:
{{{
#!CommitTicketReference repository=""
revision="8b1ff0da4b162e87edebd94e61f2cd153e9e159d"
Refs #16055 -- Deprecated
get_joining_columns()/get_reverse_joining_columns() methods.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16055#comment:28>