Implement an EnumType for MySQL/MariaDB #6536

derrabus · 2024-10-09T15:40:25Z

Q	A
Type	feature
Fixed issues	doctrine/migrations#1441 (partly)

Summary

This PR adds an EnumType that allows us to introspect and diff tables that make use of MySQL's ENUM column type.

src/Platforms/AbstractPlatform.php

berkut1 · 2024-10-09T16:38:55Z

I'm just curious, if you've decided to implement partial support for ENUM only for certain DBMSs, then why not reconsider my previous idea doctrine/migrations#1441 (comment), which theoretically shouldn't be limited to just the ENUM type?

I know you rejected this idea here doctrine/migrations#1441 (comment), but to me, it seems like the simplest and most flexible solution to implement. However, I might be missing something.

stof · 2024-10-09T16:42:30Z

@berkut1 the VerbatimType to be used during schema introspection solves a different need that having an EnumType in DBAL (even though there is indeed some overlap when considering the case of introspecting a database containing an enum field).

src/Schema/MySQLSchemaManager.php

derrabus · 2024-10-09T17:53:24Z

What @stof said, basically. I still believe a verbatim type is useful because I don't want to continuously catch up with obscure types that anyone might be using out there. But MySQL enums are pretty popular and I've seen so many workarounds in the wild that I felt like we should do something in the core to accommodate projects that rely on ENUM columns.

michnovka · 2024-10-10T11:50:29Z

src/Types/EnumType.php

+
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+
+final class EnumType extends Type


Why do you make the class final? Other types are not final

Why would you want to extend it?

Well e.g. my custom enum types could extend it, and then it'd be clear hierarchy, that this is an enum related type. Its not critical, but there is just one other type (AsciiStringType) that is final, so I don't see reason why limit the inheritance specifically for EnumType, all other types are open.

You don't need that hierarchy. This class contains a single method which is just a delegate to a platform method. Extending this class is absolutely pointless.

And we can discuss finalizing the other types for the sake of consistency as well, if you like. 😉

GromNaN · 2024-10-10T11:56:35Z

src/Platforms/AbstractPlatform.php

+            throw ColumnValuesRequired::new($this, 'ENUM');
+        }
+
+        return $this->getStringTypeDeclarationSQL(['length' => max(...array_map(mb_strlen(...), $column['values']))]);


max accepts an array.

Suggested change

return $this->getStringTypeDeclarationSQL(['length' => max(...array_map(mb_strlen(...), $column['values']))]);

return $this->getStringTypeDeclarationSQL(['length' => max(array_map(mb_strlen(...), $column['values']))]);

It does, but why's that better than using the ... operator? 🤔

3 bytes less 😅
That generates some less OPcode operations. https://3v4l.org/GjuE7/vld

* 4.2.x: Implement an EnumType for MySQL/MariaDB (doctrine#6536) Leverage the new PDO subclasses (doctrine#6532) PHPStan 1.12.6 (doctrine#6535)

With `doctrine/dbal 4.2.0` the `enum` has been added directly in DBAL, implementing it slightly different than the original TYPO3 implementation, which has been mitigated with #105279 and adopted for the `set` type in advance with #105294. TYPO3 still needs to adopt the options reading existing database schema provided by a trait to the extended platform classes and missed to pass the `enum` and `set` option values as `values` to the `Column` objects instead as `platformOption` key, hidden by the fact that getting the array representation of a column merges the platformOptions on the same level with the direct value options. Upcoming `doctrine/dbal` introduces more phpdoc-block array shapes, narrowing down possible (allowed) array keys for `platformOptions` and failing with phpstan in core tests while not beeing a technical issue. This change adjusts related code within the `ext_tables.sql` related `TableBuilder` and the platform trait reading the values and setting it to `Column` objects as value property instead of `platformOption`. Considered as task and not as bugfix due to the fact that this is only static code analysis related and not technically breaking. Minor left-over in the `SetType` implementation for `doctrine/dbal 4.0` is removed along the way. Tested and extracted from doctrine/dbal 4.3.x test, see: https://review.typo3.org/c/Packages/TYPO3.CMS/+/88596 [1] doctrine/dbal#6536 [2] https://github.com/doctrine/dbal/releases/tag/4.2.0 Resolves: #106486 Related: #105294 Related: #105279 Releases: main, 13.4 Change-Id: Ib40d8d684fdce840e4050387416227d56e39698b Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/88905 Tested-by: core-ci <[email protected]> Reviewed-by: Georg Ringer <[email protected]> Reviewed-by: Garvin Hicking <[email protected]> Tested-by: Garvin Hicking <[email protected]> Tested-by: Georg Ringer <[email protected]>

With `doctrine/dbal 4.2.0` the `enum` has been added directly in DBAL, implementing it slightly different than the original TYPO3 implementation, which has been mitigated with #105279 and adopted for the `set` type in advance with #105294. TYPO3 still needs to adopt the options reading existing database schema provided by a trait to the extended platform classes and missed to pass the `enum` and `set` option values as `values` to the `Column` objects instead as `platformOption` key, hidden by the fact that getting the array representation of a column merges the platformOptions on the same level with the direct value options. Upcoming `doctrine/dbal` introduces more phpdoc-block array shapes, narrowing down possible (allowed) array keys for `platformOptions` and failing with phpstan in core tests while not beeing a technical issue. This change adjusts related code within the `ext_tables.sql` related `TableBuilder` and the platform trait reading the values and setting it to `Column` objects as value property instead of `platformOption`. Considered as task and not as bugfix due to the fact that this is only static code analysis related and not technically breaking. Minor left-over in the `SetType` implementation for `doctrine/dbal 4.0` is removed along the way. Tested and extracted from doctrine/dbal 4.3.x test, see: https://review.typo3.org/c/Packages/TYPO3.CMS/+/88596 [1] doctrine/dbal#6536 [2] https://github.com/doctrine/dbal/releases/tag/4.2.0 Resolves: #106486 Related: #105294 Related: #105279 Releases: main, 13.4 Change-Id: Ib40d8d684fdce840e4050387416227d56e39698b Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/88913 Tested-by: core-ci <[email protected]> Reviewed-by: Stefan Bürk <[email protected]> Tested-by: Stefan Bürk <[email protected]>

derrabus added New Feature MySQL MariaDB Schema Management Types Schema Introspection labels Oct 9, 2024

derrabus requested review from morozov and greg0ire October 9, 2024 15:40

greg0ire closed this Oct 9, 2024

greg0ire reopened this Oct 9, 2024

derrabus force-pushed the feature/enum-types branch 2 times, most recently from be4bcc4 to e2688d8 Compare October 9, 2024 15:57

derrabus marked this pull request as ready for review October 9, 2024 15:58

derrabus force-pushed the feature/enum-types branch from e2688d8 to be7d0e6 Compare October 9, 2024 15:59

derrabus mentioned this pull request Oct 9, 2024

Migrations always generated for custom type with DBAL 4 doctrine/migrations#1441

Open

stof reviewed Oct 9, 2024

View reviewed changes

src/Platforms/AbstractPlatform.php Outdated Show resolved Hide resolved

src/Platforms/AbstractPlatform.php Outdated Show resolved Hide resolved

stof reviewed Oct 9, 2024

View reviewed changes

src/Schema/MySQLSchemaManager.php Outdated Show resolved Hide resolved

derrabus force-pushed the feature/enum-types branch 2 times, most recently from 2bb4731 to 2f81998 Compare October 9, 2024 17:01

derrabus force-pushed the feature/enum-types branch 4 times, most recently from 0f5cfd2 to 3c462e4 Compare October 10, 2024 08:44

derrabus added this to the 4.2.0 milestone Oct 10, 2024

derrabus requested a review from stof October 10, 2024 09:46

derrabus force-pushed the feature/enum-types branch 2 times, most recently from cf3001f to 3811651 Compare October 10, 2024 09:59

Implement an EnumType for MySQL/MariaDB

ad34f74

derrabus force-pushed the feature/enum-types branch from 3811651 to ad34f74 Compare October 10, 2024 10:16

morozov approved these changes Oct 10, 2024

View reviewed changes

michnovka reviewed Oct 10, 2024

View reviewed changes

GromNaN reviewed Oct 10, 2024

View reviewed changes

derrabus merged commit 9744af4 into doctrine:4.2.x Oct 10, 2024
91 of 92 checks passed

derrabus deleted the feature/enum-types branch October 10, 2024 12:20

derrabus added a commit to derrabus/dbal that referenced this pull request Oct 10, 2024

Merge branch '4.2.x' into 5.0.x

ef19927

* 4.2.x: Implement an EnumType for MySQL/MariaDB (doctrine#6536) Leverage the new PDO subclasses (doctrine#6532) PHPStan 1.12.6 (doctrine#6535)

derrabus mentioned this pull request Oct 10, 2024

restore comment hint (useful for enumtype declaration) #6444

Closed

michnovka mentioned this pull request Oct 10, 2024

Bump DBAL 4 min version to 4.2.0 Elao/PhpEnums#234

Closed

derrabus mentioned this pull request Oct 12, 2024

Auto-detect values for EnumType columns doctrine/orm#11666

Merged

alcohol mentioned this pull request Jan 23, 2025

Symfony uuid phpstan/phpstan-doctrine#637

Closed

janedbal mentioned this pull request Apr 24, 2025

Add missing descriptors for SmallFloatType and EnumType phpstan/phpstan-doctrine#656

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement an EnumType for MySQL/MariaDB #6536

Implement an EnumType for MySQL/MariaDB #6536

derrabus commented Oct 9, 2024

berkut1 commented Oct 9, 2024

stof commented Oct 9, 2024 •

edited

Loading

derrabus commented Oct 9, 2024

michnovka Oct 10, 2024

derrabus Oct 10, 2024

michnovka Oct 11, 2024

derrabus Oct 11, 2024

GromNaN Oct 10, 2024

derrabus Oct 10, 2024

GromNaN Oct 10, 2024


		use Doctrine\DBAL\Platforms\AbstractPlatform;

		final class EnumType extends Type

	return $this->getStringTypeDeclarationSQL(['length' => max(...array_map(mb_strlen(...), $column['values']))]);
	return $this->getStringTypeDeclarationSQL(['length' => max(array_map(mb_strlen(...), $column['values']))]);

Implement an EnumType for MySQL/MariaDB #6536

Implement an EnumType for MySQL/MariaDB #6536

Conversation

derrabus commented Oct 9, 2024

Summary

berkut1 commented Oct 9, 2024

stof commented Oct 9, 2024 • edited Loading

derrabus commented Oct 9, 2024

michnovka Oct 10, 2024

Choose a reason for hiding this comment

derrabus Oct 10, 2024

Choose a reason for hiding this comment

michnovka Oct 11, 2024

Choose a reason for hiding this comment

derrabus Oct 11, 2024

Choose a reason for hiding this comment

GromNaN Oct 10, 2024

Choose a reason for hiding this comment

derrabus Oct 10, 2024

Choose a reason for hiding this comment

GromNaN Oct 10, 2024

Choose a reason for hiding this comment

stof commented Oct 9, 2024 •

edited

Loading