Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#35032 closed Bug (fixed)

Cannot save record with UUID field after migrating existing UUIDField created in Django < 5.0

Reported by: Emanuel Andrecut Owned by: Emanuel Andrecut
Component: Database layer (models, ORM) Version: 5.0
Severity: Release blocker Keywords: uuid, mariadb, UUIDField, DataError
Cc: raydeal Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django 5.0 release notes specifies that on MariaDB 10.7+, UUIDField is now created as UUID column rather than CHAR(32) column and migrations where "char(32)" is hardcoded must be added to keep compatibility for UUIDField<s> created before version 5.0: https://docs.djangoproject.com/en/5.0/releases/5.0/#migrating-existing-uuidfield-on-mariadb-10-7

However after applying such migrations as described in the release notes records cannot be saved anymore:

  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/query.py", line 953, in get_or_create
    return self.create(**params), True
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/query.py", line 677, in create
    obj.save(force_insert=True, using=self.db)
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/base.py", line 814, in save
    self.save_base(
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/base.py", line 901, in save_base
    updated = self._save_table(
              ^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/base.py", line 1059, in _save_table
    results = self._do_insert(
              ^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/base.py", line 1100, in _do_insert
    return manager._insert(
           ^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/query.py", line 1845, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1822, in execute_sql
    cursor.execute(sql, params)
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 79, in execute
    return self._execute_with_wrappers(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 92, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 100, in _execute
    with self.db.wrap_database_errors:
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 105, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/django/db/backends/mysql/base.py", line 75, in execute
    return self.cursor.execute(query, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/MySQLdb/cursors.py", line 179, in execute
    res = self._query(mogrified_query)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/manu/venv/lib/python3.11/site-packages/MySQLdb/cursors.py", line 330, in _query
    db.query(q)
  File "/home/manu/venv/lib/python3.11/site-packages/MySQLdb/connections.py", line 255, in query
    _mysql.connection.query(self, query)
django.db.utils.DataError: (1406, "Data too long for column 'id' at row 1")

That is because, before this commit https://github.com/django/django/commit/7cd187a5ba58d7769039f487faeb9a5a2ff05540, has_native_uuid_field would return False and after the 5.0 change, saving an UUID for old fields that must still use CHAR(32) will not try to save it using the hex value of UUID: https://github.com/django/django/blob/main/django/db/models/fields/__init__.py#L2737

Change History (6)

comment:2 by Mariusz Felisiak, 11 months ago

Cc: raydeal added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks for the report! Sounds reasonable, I'm not sure if we want to add workarounds to the Django itself. What do you think about updating release notes? For example:

  • docs/releases/5.0.txt

    diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt
    index 4c86337b74..87fda8d866 100644
    a b Django < 5.0 should be replaced with a ``UUIDField`` subclass backed by (this hunk was shorter than expected)  
    508508        def db_type(self, connection):
    509509            return "char(32)"
     510
     511        def get_db_prep_value(self, value, connection, prepared=False):
     512            value = super().get_db_prep_value(value, connection, prepared)
     513            if value is not None:
     514                value = value.hex
     515            return value
     516
    510517For example::
    511518
    512519    class MyModel(models.Model):
    For example::  
    516523Should become::
    517524
    518525    class Char32UUIDField(models.UUIDField):
    519         def db_type(self, connection):
    520             return "char(32)"
     526        ...
    521527
    522528
    523529    class MyModel(models.Model):

Regression in 7cd187a5ba58d7769039f487faeb9a5a2ff05540.

Last edited 11 months ago by Mariusz Felisiak (previous) (diff)

in reply to:  2 ; comment:3 by Emanuel Andrecut, 11 months ago

I've dropped the Django changes in the PR and updated class Char32UUIDField in release notes, please check.

Replying to Mariusz Felisiak:

Thanks for the report! Sounds reasonable, I'm not sure if we want to add workarounds to the Django itself. What do you think about updating release notes? For example:

  • docs/releases/5.0.txt

    diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt
    index 4c86337b74..87fda8d866 100644
    a b Django < 5.0 should be replaced with a ``UUIDField`` subclass backed by (this hunk was shorter than expected)  
    508508        def db_type(self, connection):
    509509            return "char(32)"
     510
     511        def get_db_prep_value(self, value, connection, prepared=False):
     512            value = super().get_db_prep_value(value, connection, prepared)
     513            if value is not None:
     514                value = value.hex
     515            return value
     516
    510517For example::
    511518
    512519    class MyModel(models.Model):
    For example::  
    516523Should become::
    517524
    518525    class Char32UUIDField(models.UUIDField):
    519         def db_type(self, connection):
    520             return "char(32)"
     526        ...
    521527
    522528
    523529    class MyModel(models.Model):

Regression in 7cd187a5ba58d7769039f487faeb9a5a2ff05540.

in reply to:  3 comment:4 by Mariusz Felisiak, 11 months ago

Has patch: set
Owner: changed from nobody to Emanuel Andrecut
Status: newassigned
Triage Stage: AcceptedReady for checkin

Replying to Emanuel Andrecut:

I've dropped the Django changes in the PR and updated class Char32UUIDField in release notes, please check.

Looks good, thanks.

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 11 months ago

Resolution: fixed
Status: assignedclosed

In e72b282:

Fixed #35032 -- Corrected Char32UUIDField implementation in 5.0 release notes.

This fixes Char32UUIDField implementation in 5.0 release notes causing
records with UUIDFields created using pre-Django 5.0 and CHAR(32) not
being able to be saved anymore after upgrading and keeping the CHAR(32)
columns.

Regression in 7cd187a5ba58d7769039f487faeb9a5a2ff05540.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 11 months ago

In 636d701:

[5.0.x] Fixed #35032 -- Corrected Char32UUIDField implementation in 5.0 release notes.

This fixes Char32UUIDField implementation in 5.0 release notes causing
records with UUIDFields created using pre-Django 5.0 and CHAR(32) not
being able to be saved anymore after upgrading and keeping the CHAR(32)
columns.

Regression in 7cd187a5ba58d7769039f487faeb9a5a2ff05540.

Backport of e72b2826ff1eaf2f48ee54a40d2f2988a1fdbb0a from main

Note: See TracTickets for help on using tickets.
Back to Top