Skip to content

Conversation

Abdeldjalil-H
Copy link
Contributor

@Abdeldjalil-H Abdeldjalil-H commented Feb 14, 2025

Description

Added MySQL-specific implementation for CharEnumField to properly support native ENUM types in MySQL databases. This implementation:

  • Introduces a new db_mysql class for CharEnumFieldInstance
  • Converts enum values to MySQL ENUM type definition
  • Properly escapes enum values to prevent SQL injection
  • Maintains type safety between Python Enum and MySQL ENUM

Motivation and Context

Currently, CharEnumField doesn't utilize MySQL's native ENUM type, instead storing enum values as regular VARCHAR fields. This change improves:

  • Data integrity by enforcing enum constraints at the database level
  • Query performance by using MySQL's optimized ENUM type
  • Schema clarity by explicitly showing valid enum values in database structure

How Has This Been Tested?

make test

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

codspeed-hq bot commented Feb 14, 2025

CodSpeed Performance Report

Merging #1883 will not alter performance

Comparing Abdeldjalil-H:feat/mysql-enum-field-support (f63bd6b) with develop (4e47cff)

Summary

✅ 16 untouched benchmarks

@Abdeldjalil-H
Copy link
Contributor Author

Partially addresses #937

Copy link
Contributor

@henadzit henadzit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! However, I have multiple concerns about this change:

  • This is a breaking change
  • This is not supported by aerich which is used by a lot of people
  • This change will make the MySQL integration standout

If we decide to use the database enums, I think it should be a more complex change:

  • Implement this for all databases that support enums
  • Update aerich to support enums
  • Make sure aerich can handle the migration to enums

Potentially we might have a configuration option on a field to regulate whether to use database enums to smooth the migration.

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.

2 participants