indexing content_type and object_pk
crolfe opened this issue · 6 comments
Hello!
I have encountered a major performance issue with this library, due to the generic relation defined in BaseCommentAbstractModel
I was hoping it'd be as simple as adding this to the Meta:
index_together = [
('content_type', 'object_pk')
]
Alas, this does not work on MySQL (not sure if it would or wouldn't on Postgres or other databases), due to the use of a TextField
for the object_pk
:
_mysql_exceptions.OperationalError: (1170, "BLOB/TEXT column 'object_pk' used in key specification without a key length")
I have already solved this for my own project's needs in a fork by switching to a CharField
, but I suspect the performance hit (of the missing index) is something other users will encounter, sooner or later, so I'd like to push it upstream.
The major risk I can see to this is it would be a breaking change for anyone who has object_pk
s larger than the max_length of a CharField
, but the stated intention for a non-numeric field is to support UUIDs, which could still be supported.
I will submit a PR if this something that would be accepted.
+1
Note that the pull request django/django#9132 is going a step forward allowing indexes on long MySQL fields.
@claudep Good to know that's coming, thanks!
Do you know approximately when that might land in Django? Also, would 1.11 get that that change?
Absolutely no chance for such a patch being backported, sorry.
Don't hesitate to review/comment the patch and weigh on the ticket if you want to speed up things.