django-mptt/django-mptt

TypeError: unsupported operand type(s) for +=: 'set' and 'tuple

johnnyporkchops opened this issue · 7 comments

When upgrading from Django-mptt 0.8.6 to 0.11.0 or later version, Django migrations fail with unsupported type error:
TypeError: unsupported operand type(s) for +=: 'set' and 'tuple'
The error is traced to:

cls._meta.index_together += (index_together,)

The code is expecting a tuple so when a set is passed, it throws this error. Is it possible to make the code more robust to handle both sets and tuples?

We came across this error when upgrading from Django 1.11 to 2.2.24 which required mptt upgrade from 0.86 to 0.11 due to template loader errors. This error did not occur before upgrading.

Failing migrations stack trace:

(20211018-eregs-env) macadmins-mbp-5:fec-eregs pkasireddy$ python manage.py migrate
System check identified some issues:

WARNINGS:
regcore.Diff: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
regcore.Layer: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
regcore.NoticeCFRPart: (models.W042) Auto-created primary key used when not defining a primary key type, by default 'django.db.models.AutoField'.
	HINT: Configure the DEFAULT_AUTO_FIELD setting or the AppConfig.default_auto_field attribute to point to a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
Operations to perform:
 Apply all migrations: auth, contenttypes, regcore, regulations, sessions
Running migrations:
 Applying contenttypes.0001_initial... OK
 Applying contenttypes.0002_remove_content_type_name... OK
 Applying auth.0001_initial... OK
 Applying auth.0002_alter_permission_name_max_length... OK
 Applying auth.0003_alter_user_email_max_length... OK
 Applying auth.0004_alter_user_username_opts... OK
 Applying auth.0005_alter_user_last_login_null... OK
 Applying auth.0006_require_contenttypes_0002... OK
 Applying auth.0007_alter_validators_add_error_messages... OK
 Applying auth.0008_alter_user_username_max_length... OK
 Applying auth.0009_alter_user_last_name_max_length... OK
 Applying auth.0010_alter_group_name_max_length... OK
 Applying auth.0011_update_proxy_permissions... OK
 Applying auth.0012_alter_user_first_name_max_length... OK
 Applying regcore.0001_initial... OK
 Applying regcore.0002_mptt_add_fields... OK
 Applying regcore.0003_mptt_copy_children...Traceback (most recent call last):
 File "manage.py", line 10, in <module>
  execute_from_command_line(sys.argv)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
  utility.execute()
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/core/management/__init__.py", line 413, in execute
  self.fetch_command(subcommand).run_from_argv(self.argv)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/core/management/base.py", line 354, in run_from_argv
  self.execute(*args, **cmd_options)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/core/management/base.py", line 398, in execute
  output = self.handle(*args, **options)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/core/management/base.py", line 89, in wrapped
  res = handle_func(*args, **kwargs)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/core/management/commands/migrate.py", line 246, in handle
  fake_initial=fake_initial,
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/db/migrations/executor.py", line 117, in migrate
  state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/db/migrations/executor.py", line 147, in _migrate_all_forwards
  state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/db/migrations/executor.py", line 227, in apply_migration
  state = migration.apply(state, schema_editor)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/db/migrations/migration.py", line 126, in apply
  operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/django/db/migrations/operations/special.py", line 190, in database_forwards
  self.code(from_state.apps, schema_editor)
 File "/Users/pkasireddy/.pyenv/versions/3.7.10/envs/20211018-eregs-env/src/regcore/regcore/migrations/0003_mptt_copy_children.py", line 17, in rebuild
  mptt.register(Regulation)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/mptt/__init__.py", line 13, in register
  return MPTTModelBase.register(*args, **kwargs)
 File "/Users/pkasireddy/.pyenv/versions/20211018-eregs-env/lib/python3.7/site-packages/mptt/models.py", line 352, in register
  cls._meta.index_together += (index_together,)
TypeError: unsupported operand type(s) for +=: 'set' and 'tuple



This code could certainly be made more resilient by converting both sides of the addition into an expected format.

Maybe one could use the undocumented normalize_together method in django/db/models/options.py to help with this.

Thank you for fielding this issue and for the suggestion. We were able to import normalize_together into a model and attempt to apply it to existing index_together tuples, but this did not resolve the error, although our usage of this undocumented utility may have been syntactically incorrect.
from django.db.models.options import (normalize_together)
index_together = normalize_together(index_together)

Update: the below actually errors upon migrating, see #803 (comment) below for final solution
However, we were able to resolve the unsupported operand error by removing migration operations in previous migration files that were manually added by previous code maintainers, like:

# migrations.AlterIndexTogether(
       #     name='regulation',
       #     index_together=set([('version', 'label_string')]),
       # )

My guess was that converting to a set was needed to resolve a similar error that's now fixed in newer versions of dango-mptt (?) But I could not find anything in changelogs that supports that theory.

django-mptt only recently started using index_together (in 2017, that is). Previously it added indices on tree_id, lft, rght and level; these indices were more expensive and less useful than a combined index on tree_id and lft.

Maybe that's the reason for this?

That is likely since we had been using django-mptt version 0.8.6 up until now, which seems to date to @ 2016

It turns out the alterIndexTogether operations were not added manually as mentioned above. Rather, Django auto-generates a migration operation to convert index_together or unique_together to a set regardless of whether they are written in a list, or tuple in a model's class Meta. However, when just commenting out those operations to get mptt.register() to run, Django still wants to create another migration (upon running makemigrations) at the end to convert them to a set, which then errors upon migrating with " relation already/does not exist" error.

So we finally solved it by converting to a tuple in the migration file's functions, before mptt.register() runs.
In our case we did :

def convert_to_tuplet(set):
    return tuple(i for i in set)
  ...
def rebuild(apps, schema_editor):
  ...
    Document = apps.get_model('regcore', 'Document')
    Document._meta.index_together = convert_to_tuple(Document._meta.index_together)
 ...
    mptt.register(Document)

Hope this helps anyone who might come up against this seemingly obscure issue.

I'm not sure whether there's anything that could be done in django-mptt to help with this?

We were originally monkey-patching dajngo-mptt's models.py to convert the reference to our app's index_together to a tuple to "normalize" it with django-mptt's index_together when it does the merging of the two with += here.

def convert_to_tuplet(set):
    return tuple(i for i in set)

(Line 377 dango-mptt models.py)

                convert_to_tuple(cls._meta.index_together) += (index_together,)

Would it work to make mptt's index_together be passed as a set rather than tuple? I admittedly do not know enough about how django-mptt (or mptt in general) works to know if this has potential to break something somewhere else or for somebody else.
We are happy to see this issue closed, considering this documented workaround for an issue that may never come up for others users of django-mptt.