Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update models.py #1748

Closed
wants to merge 1 commit into from
Closed

Update models.py #1748

wants to merge 1 commit into from

Conversation

shariq1010
Copy link

task_id = sa.Column(sa.String(255), unique=True)
Th above statement is not executing on MySQL saying "(1071, 'Specified key was too long; max key length is 767 bytes')"

reducing the task_id to 155 char is working fine

`task_id = sa.Column(sa.String(255), unique=True)`
Th above statement is not executing on MySQL saying "(1071, 'Specified key was too long; max key length is 767 bytes')"

reducing the task_id to 155 char is working fine
@ask
Copy link
Contributor

ask commented Dec 18, 2013

Thanks! But this is a backward incompatible change and can first be included in version 3.2.

@shariq1010
Copy link
Author

Thanks Ask Solem Hoel for taking the note on this.
I was facing the problem on setting up my system and did the changes in my local system, then though of doing the same for all :)

@ionelmc ionelmc added this to the v3.2.0 milestone Feb 9, 2014
@ask ask closed this in fa18204 Mar 5, 2016
@adamn
Copy link

adamn commented Aug 3, 2016

What's the rationale for 155 here? 255 should be fine (255*3 < 767) since the max width of a utf8 char is 3.

http://dev.mysql.com/doc/refman/5.6/en/create-table.html

In addition, MySQL 5.6 has support for larger indexes:

http://dev.mysql.com/doc/refman/5.6/en/innodb-parameters.html#sysvar_innodb_large_prefix

To be honest, we should probably use varbinary here since task_id shouldn't have a character set or collation:

http://docs.sqlalchemy.org/en/latest/core/type_basics.html#sqlalchemy.types.VARBINARY
http://dev.mysql.com/doc/refman/5.7/en/binary-varbinary.html

@ask
Copy link
Contributor

ask commented Aug 4, 2016

That sounds like a good idea, maybe sql has an UUIDField already, or some example implementation.

Technically the task_id can contain any value though, but I cannot remember having promised unicode support ;)

@adamn
Copy link

adamn commented Aug 4, 2016

MySQL doesn't have a UUID field ;-)

It would be awesome if SqlAlchemy had a portable UUID field like Django so we didn't have to think about this.

  1. Django's ORM uses char(32) for the UUID field on MySQL and does transformation in and out of the database. It uses a real UUID field on Postgres.
  2. Do people have an expectation of task_id being > 32 characters?
  3. If not and we can really just say that task_ids will always be a UUID from now on then binary(16) seems to be fastest and supported on all systems. String(32) would be more flexible potentially and consistent with Django's implementation.

@ask
Copy link
Contributor

ask commented Aug 9, 2016

You can have natural task_ids, so they can be any string, but I doubt that is used much. There's no documentation on the max length, but I'm sure we could get away with 64 (32 is maybe stretching it).

I guess it could be configurable so people who really need natural ids can set a flag for it to be String(64), otherwise it's Binary(16) by default.

@ask
Copy link
Contributor

ask commented Aug 9, 2016

Optimally we should also replace the PickleType with a simple TextField, as the BaseBackend will already deal with serialization (and it's configurable using the result_serializer config).

In the new django-celery-results I did this, so the serialization is handled by the backend and the model has new content_type and content_encoding columns. For serializer='pickle' the content is bas64 encoded to fit into a textfield.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants