Implement an EnumType for MySQL/MariaDB #6536
Conversation
derrabus
force-pushed
the
feature/enum-types
branch
2 times, most recently
from
be4bcc4
to
e2688d8
Compare
derrabus
force-pushed
the
feature/enum-types
branch
from
e2688d8
to
be7d0e6
Compare
|
I'm just curious, if you've decided to implement partial support for 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. |
|
@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). |
derrabus
force-pushed
the
feature/enum-types
branch
2 times, most recently
from
2bb4731
to
2f81998
Compare
|
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. |
derrabus
force-pushed
the
feature/enum-types
branch
4 times, most recently
from
0f5cfd2
to
3c462e4
Compare
derrabus
force-pushed
the
feature/enum-types
branch
2 times, most recently
from
cf3001f
to
3811651
Compare
derrabus
force-pushed
the
feature/enum-types
branch
from
3811651
to
ad34f74
Compare
|
|
||
| use Doctrine\DBAL\Platforms\AbstractPlatform; | ||
|
|
||
| final class EnumType extends Type |
There was a problem hiding this comment.
Why do you make the class final? Other types are not final
There was a problem hiding this comment.
Why would you want to extend it?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. 😉
| throw ColumnValuesRequired::new($this, 'ENUM'); | ||
| } | ||
|
|
||
| return $this->getStringTypeDeclarationSQL(['length' => max(...array_map(mb_strlen(...), $column['values']))]); |
There was a problem hiding this comment.
max accepts an array.
| return $this->getStringTypeDeclarationSQL(['length' => max(...array_map(mb_strlen(...), $column['values']))]); | |
| return $this->getStringTypeDeclarationSQL(['length' => max(array_map(mb_strlen(...), $column['values']))]); |
There was a problem hiding this comment.
It does, but why's that better than using the ... operator? 🤔
There was a problem hiding this comment.
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)
Summary
This PR adds an
EnumTypethat allows us to introspect and diff tables that make use of MySQL'sENUMcolumn type.