diff --git a/docker-compose.yml b/docker-compose.yml index a77e338f131e..569edb5be651 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -132,8 +132,6 @@ services: - --skip-name-resolve # Disable performance schema for faster startup - --performance-schema=OFF - # Allow nonstandard FKs (needed for translations) - - --skip-restrict-fk-on-non-standard-key healthcheck: test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "--silent"] start_interval: 1s diff --git a/docs/topics/development/localization_and_internationalization.md b/docs/topics/development/localization_and_internationalization.md index e53db67b719e..9a215dfd264a 100644 --- a/docs/topics/development/localization_and_internationalization.md +++ b/docs/topics/development/localization_and_internationalization.md @@ -85,7 +85,13 @@ models.signals.pre_save.connect(save_signal, ### How It Works Behind the Scenes -A `TranslatedField` is actually a `ForeignKey` to the `translations` table. To support multiple languages, we use a special feature of MySQL allowing a `ForeignKey` to point to multiple rows. +Rows in the translations table have an `autoid` primary key and an `id` key that is +shared between different translations of the same string in multiple languages. + +That `id` is generated from a dedicated sequence table `translations_seq` every time +a new string to be translated is saved. + +A `TranslatedField` is actually a `ForeignKey` to the `translations` table. To support multiple languages, these have `db_constraint` set to `False`, and the FK points to the `id` key. #### When Querying @@ -94,7 +100,7 @@ Our base manager has a `_with_translations()` method that is automatically calle - Adds an extra `lang=lang` in the query to prevent query caching from returning objects in the wrong language. - Calls `olympia.translations.transformers.get_trans()` which builds a custom SQL query to fetch translations in the current language and fallback language. -This custom query ensures that only the specified languages are considered and uses a double join with `IF`/`ELSE` for each field. The results are fetched using a slave database connection to improve performance. +This custom query ensures that only the specified languages are considered and uses a double join with `IF`/`ELSE` for each field. The results are fetched using a replica database connection to improve performance. #### When Setting diff --git a/src/olympia/translations/fields.py b/src/olympia/translations/fields.py index 93a1cbc58bab..790bf989e346 100644 --- a/src/olympia/translations/fields.py +++ b/src/olympia/translations/fields.py @@ -127,12 +127,15 @@ class TranslatedField(models.ForeignKey): forward_related_accessor_class = TranslationDescriptor def __init__(self, **kwargs): - # to_field: The field on the related object that the relation is to. - # Django wants to default to translations.autoid, but we need id. kwargs.update( { 'null': True, + # Field on the related object that the relation is to. Django + # wants to default to translations.autoid, but we need id. 'to_field': 'id', + # No db constraint, it's not a "true" FK, there can be multiple + # rows for a given translated string, one for each locale. + 'db_constraint': False, 'unique': True, 'blank': True, 'on_delete': models.SET_NULL, diff --git a/src/olympia/translations/migrations/0004_remove_legacy_constraints.py b/src/olympia/translations/migrations/0004_remove_legacy_constraints.py new file mode 100644 index 000000000000..211e5b340459 --- /dev/null +++ b/src/olympia/translations/migrations/0004_remove_legacy_constraints.py @@ -0,0 +1,48 @@ +# Generated by Django 5.2.13 on 2026-04-28 21:26 + +from django.db import migrations, router + + +def remove_legacy_translations_constraints(apps, schema_editor): + """ + Remove Foreign Key constraints to Translations.id - TranslatedField should + now be using db_constraint: False as there can be multiple translation rows + sharing the same id, one per locale per translated string. + """ + Translation = apps.get_model('translations', 'Translation') + models_related_to_translations = { + f.related_model + for f in Translation._meta.get_fields(include_hidden=True) + if f.auto_created and not f.concrete and (f.one_to_one or f.one_to_many) + } + connection = schema_editor.connection + with connection.cursor() as cursor: + for model in models_related_to_translations: + constraints = connection.introspection.get_constraints( + cursor, model._meta.db_table + ) + for name, info in constraints.items(): + if info['foreign_key'] == (Translation._meta.db_table, 'id'): + schema_editor.execute( + schema_editor._delete_fk_sql(model, name), params=None + ) + + +class Migration(migrations.Migration): + # We're going to execute ALTER TABLE statements that are not supported + # inside a transaction, so disable transaction management to avoid + # "Executing DDL statements while in a transaction on databases that can't + # perform a rollback is prohibited" error. + atomic = False + + dependencies = [ + ('translations', '0003_puretranslation_purifiedmarkdowntranslation'), + # Depend on these apps as they use TranslatedField + ('addons', '0060_backfill_last_content_review_status'), + ('versions', '0052_delete_enable_source_builder_waffle_switch'), + ('bandwagon', '0010_fix_default_locale_spanish'), + ] + + operations = [ + migrations.RunPython(remove_legacy_translations_constraints, lambda *args: None) + ] diff --git a/src/olympia/translations/models.py b/src/olympia/translations/models.py index f77422581370..95f71a850e2f 100644 --- a/src/olympia/translations/models.py +++ b/src/olympia/translations/models.py @@ -102,12 +102,7 @@ def delete(self, using=None): assert self._get_pk_val() is not None collector = Collector(using=using) collector.collect([self], collect_related=False) - # In addition, because we have FK pointing to a non-unique column, - # we need to force MySQL to ignore constraints because it's dumb - # and would otherwise complain even if there are remaining rows - # that matches the FK. - with connections[using].constraint_checks_disabled(): - collector.delete() + collector.delete() else: # If no other Translations with that id exist, then we should let # django behave normally. It should find the related model and set diff --git a/src/olympia/translations/tests/test_fields.py b/src/olympia/translations/tests/test_fields.py index 282a33c64e1c..bbee934d8c97 100644 --- a/src/olympia/translations/tests/test_fields.py +++ b/src/olympia/translations/tests/test_fields.py @@ -47,3 +47,4 @@ def test_user_foreign_key_field_deconstruct(): assert kwargs['require_locale'] == new_field_instance.require_locale assert kwargs['to'] == new_field_instance.to assert kwargs['short'] == new_field_instance.short + assert kwargs['db_constraint'] is False diff --git a/src/olympia/translations/tests/test_models.py b/src/olympia/translations/tests/test_models.py index b545d690a337..e0cff78c8a2d 100644 --- a/src/olympia/translations/tests/test_models.py +++ b/src/olympia/translations/tests/test_models.py @@ -1057,3 +1057,22 @@ def get_model(): # Check that de finds the right translation. fresh_german = get_model() assert fresh_german.name == '😀' + + +@pytest.mark.django_db +def test_no_db_constraints(): + models_related_to_translations = { + f.related_model + for f in Translation._meta.get_fields(include_hidden=True) + if f.auto_created and not f.concrete and (f.one_to_one or f.one_to_many) + } + assert models_related_to_translations + connection = connections['default'] + with connection.cursor() as cursor: + for model in models_related_to_translations: + constraints = connection.introspection.get_constraints( + cursor, model._meta.db_table + ) + for name, info in constraints.items(): + assert not name.endswith(f'{Translation._meta.db_table}_id'), name + assert info['foreign_key'] != (Translation._meta.db_table, 'id'), name