Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow DiscriminatorColumn with length=0 #9410

Merged

Conversation

bcremer
Copy link
Contributor

@bcremer bcremer commented Jan 20, 2022

This patch allows to set length to a Zero-Value on a @ORM\DiscriminatorColumn:

/**
 * @ORM\DiscriminatorColumn(
 *     name="type",
 *     type="string",
 *     length=0
 * )
 */
abstract class MyEntityClass

Right now a value <1 will be replaced by the default value (255) of a ternary operation in \Doctrine\ORM\Mapping\Driver\AnnotationDriver::loadMetadataForClass .
This seems to be an accidental results from the weak type comparison.

On a @ORM\Column a length of Zero will be parsed as is and not changed to 255.

    /**
     * @ORM\Column (
     *     name="example", 
     *     type="string", 
     *     length=0, 
     *     nullable=true
     * )
     */
    private string $example;

Background:

I'm mapping MySQL enums to strings as described here: https://www.doctrine-project.org/projects/doctrine-orm/en/2.7/cookbook/mysql-enums.html#solution-1-mapping-to-varchars

Since the 3.2 release with the new Platform-aware schema comparison (
https://www.doctrine-project.org/2021/11/26/dbal-3.2.0.html) I get a lot of diffs in our enum fields.

    /**
     * @ORM\Column (
     *   name="platform",
     *   type="string",
     *   columnDefinition="enum('iOS','Android') NULL DEFAULT",
     */
    private string $platform;

The schema diff will report different values for the length-property as the MySQl Platform yields lenght = 0 for the mysql native ENUM type.

To fix the diff the length has to be specified as 0:

    /**
     * @ORM\Column (
     *   name="platform",
     *   type="string",
     *   length=0,
     *   columnDefinition="enum('iOS','Android') NULL DEFAULT",
     */
    private string $platform;

This works for all columns and the enum fields won't generate diffs anymore.

The same lenght=0 change is required if the DiscriminatorColumn uses a native Enum type like so:

/**
 * @ORM\Table
 * @ORM\Entity
 * @ORM\InheritanceType("SINGLE_TABLE")
 * @ORM\DiscriminatorColumn(
 *     name="type",
 *     type="string",
 *     length=0,
 *     columnDefinition="enum('region','airport','station','poi') NOT NULL",
 * )
 * @ORM\DiscriminatorMap({
 *     "region" = "\Storage\MySQL\ApiV3\Entity\Poi\Region",
 *     "airport" = "\Storage\MySQL\ApiV3\Entity\Poi\Airport",
 *     "station" = "\Storage\MySQL\ApiV3\Entity\Poi\Station",
 *     "poi" = "\Storage\MySQL\ApiV3\Entity\Poi\GeneralPoi",
 * })
 */
abstract class Poi

Right now this is not possible as length=0 on a DiscriminatorColumn will become length=255 and thus a diff will be reported from the schema comparison.

@bcremer bcremer changed the title Allow DiscriminatorColumn with length=0 WIP / Allow DiscriminatorColumn with length=0 Jan 20, 2022
@bcremer bcremer changed the title WIP / Allow DiscriminatorColumn with length=0 Allow DiscriminatorColumn with length=0 Jan 20, 2022
@derrabus
Copy link
Member

Thank you. As always, please add a test. 🙃

@bcremer bcremer force-pushed the allow-length-0-in-DiscriminatorColumn branch from 0026b5f to e2846fd Compare January 21, 2022 08:17
@bcremer
Copy link
Contributor Author

bcremer commented Jan 21, 2022

Sure thing, added some testcases.

@bcremer bcremer force-pushed the allow-length-0-in-DiscriminatorColumn branch from e2846fd to 13c6f26 Compare January 21, 2022 08:19
@bcremer bcremer force-pushed the allow-length-0-in-DiscriminatorColumn branch from 13c6f26 to d20b8b8 Compare January 21, 2022 08:28
@derrabus derrabus added this to the 2.11.1 milestone Jan 21, 2022
@derrabus derrabus merged commit b596e6a into doctrine:2.11.x Jan 21, 2022
derrabus added a commit to derrabus/orm that referenced this pull request Jan 23, 2022
* 2.11.x:
  Add support for PHP 8.1 enums in embedded classes (doctrine#9419)
  Added class-string typehint on $targetEntity (doctrine#9415)
  Allow DiscriminatorColumn with length=0 (doctrine#9410)
  Move UnderscoreNamingStrategyTest to correct namespace (doctrine#9414)
derrabus added a commit that referenced this pull request Jan 23, 2022
* 2.11.x:
  Add support for PHP 8.1 enums in embedded classes (#9419)
  Added class-string typehint on $targetEntity (#9415)
  Allow DiscriminatorColumn with length=0 (#9410)
  Move UnderscoreNamingStrategyTest to correct namespace (#9414)
derrabus added a commit to derrabus/orm that referenced this pull request Jan 23, 2022
* 2.12.x:
  Add support for PHP 8.1 enums in embedded classes (doctrine#9419)
  Switch tests to the middleware logging system (doctrine#9418)
  Added class-string typehint on $targetEntity (doctrine#9415)
  Allow DiscriminatorColumn with length=0 (doctrine#9410)
  Move UnderscoreNamingStrategyTest to correct namespace (doctrine#9414)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants