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

Bug: Can't use integers as discriminator values with ORM v3 #11341

Open
imba28 opened this issue Mar 5, 2024 · 0 comments
Open

Bug: Can't use integers as discriminator values with ORM v3 #11341

imba28 opened this issue Mar 5, 2024 · 0 comments

Comments

@imba28
Copy link

imba28 commented Mar 5, 2024

Bug Report

Q A
BC Break yes
Version 3.0.x

Summary

A while ago, doctrine/dbal@0db36a9 stopped implementing type-specific handling when quoting values (Statement::quote()). Moreover, it explicitly added a string type hint. When trying to upgrade to dbal v4 / orm v3 we noticed that this breaks single table inheritance when configuring integers as discriminator values as briefly outlined in #9921.

In that case, SqlWalker passes an integer to Connection::quote() when generating the discriminator column SQL condition:

This results in a type error.

I guess since we can no longer rely on doctrine/dbal to resolve the parameters for us doctrine/orm needs to somehow consider the discriminator column's type and preprocess the SQL parameters accordingly.

Current behavior

When configuring integers as discriminator values Doctrine triggers a type error whenever the application generates queries;

[TypeError] Doctrine\DBAL\Connection::quote(): Argument #1 ($value) must be of type string, int given, called in /var/www/html/vendor/doctrine/orm/src/Query/SqlWalker.php on line 387  

How to reproduce

Here are two functional test that demonstrate the bug:

DiscriminatorColumn(type: 'integer')

<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Tests\OrmFunctionalTestCase;
use Generator;
use PHPUnit\Framework\Attributes\DataProvider;

class IntegerDiscriminatorValueTest extends OrmFunctionalTestCase
{
    protected function setUp(): void
    {
        parent::setUp();

        $this->setUpEntitySchema([
            IntegerBaseClass::class,
            IntegerFooEntity::class,
            IntegerBarEntity::class,
        ]);
    }

    public static function dqlStatements(): Generator
    {
        yield ['SELECT e FROM ' . IntegerBaseClass::class . ' e', '/WHERE g0_.type IN \(1, 2\)$/'];
        yield ['SELECT e FROM ' . IntegerFooEntity::class . ' e', '/WHERE g0_.type IN \(1\)$/'];
        yield ['SELECT e FROM ' . IntegerBarEntity::class . ' e', '/WHERE g0_.type IN \(2\)$/'];
    }

    #[DataProvider('dqlStatements')]
    public function testIntegerDiscriminatorValue(string $dql, string $expectedDiscriminatorValues): void
    {
        $query = $this->_em->createQuery($dql);
        $sql   = $query->getSQL();

        self::assertMatchesRegularExpression($expectedDiscriminatorValues, $sql);
    }
}

#[ORM\Entity]
#[ORM\Table(name: 'integer_discriminator')]
#[ORM\InheritanceType('SINGLE_TABLE')]
#[ORM\DiscriminatorColumn(name: 'type', type: 'integer')]
#[ORM\DiscriminatorMap([
    1 => IntegerFooEntity::class,
    2 => IntegerBarEntity::class,
])]
class IntegerBaseClass
{
    #[ORM\Id]
    #[ORM\GeneratedValue(strategy: 'IDENTITY')]
    #[ORM\Column(type: 'integer')]
    private int|null $id = null;
}

#[ORM\Entity]
class IntegerFooEntity extends IntegerBaseClass
{
}

#[ORM\Entity]
class IntegerBarEntity extends IntegerBaseClass
{
}

DiscriminatorColumn(type: 'string')

<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Tests\OrmFunctionalTestCase;
use Generator;
use PHPUnit\Framework\Attributes\DataProvider;

class StringIntegerEnumDiscriminatorValueTest extends OrmFunctionalTestCase
{
    protected function setUp(): void
    {
        parent::setUp();

        $this->setUpEntitySchema([
            IntegerBaseClass::class,
            IntegerFooEntity::class,
            IntegerBarEntity::class,
        ]);
    }

    public static function dqlStatements(): Generator
    {
        yield ['SELECT e FROM ' . IntegerBaseClass::class . ' e', '/WHERE g0_.type IN \(\'1\', \'2\'\)$/'];
        yield ['SELECT e FROM ' . IntegerFooEntity::class . ' e', '/WHERE g0_.type IN \(\'1\'\)$/'];
        yield ['SELECT e FROM ' . IntegerBarEntity::class . ' e', '/WHERE g0_.type IN \(\'2\'\)$/'];
    }

    #[DataProvider('dqlStatements')]
    public function testIntegerDiscriminatorValue(string $dql, string $expectedDiscriminatorValues): void
    {
        $query = $this->_em->createQuery($dql);
        $sql   = $query->getSQL();

        self::assertMatchesRegularExpression($expectedDiscriminatorValues, $sql);
    }
}

#[ORM\Entity]
#[ORM\Table(name: 'numeric_string_discriminator')]
#[ORM\InheritanceType('SINGLE_TABLE')]
#[ORM\DiscriminatorColumn(name: 'type', type: 'string')]
#[ORM\DiscriminatorMap([
    '1' => EnumFooEntity::class,
    '2' => EnumBarEntity::class,
])]
class EnumBaseClass
{
    #[ORM\Id]
    #[ORM\GeneratedValue(strategy: 'IDENTITY')]
    #[ORM\Column(type: 'integer')]
    private int|null $id = null;
}

#[ORM\Entity]
class EnumFooEntity extends IntegerBaseClass
{
}

#[ORM\Entity]
class EnumBarEntity extends IntegerBaseClass
{
}

Expected behavior

  1. When configuring a discriminator column of type integer I'd expect all queries to contain unquoted parameters: WHERE type IN (1, 2)

    Example

    #[ORM\DiscriminatorColumn(name: 'type', type: 'integer')]
    #[ORM\DiscriminatorMap([
        1 => IntegerFooEntity::class,
        2 => IntegerBarEntity::class,
    ])]
  2. However, If I changed the type to string I'd like to see the parameters to be quoted instead even if the discriminator values contain numeric strings: WHERE type IN ('1', '2'). This should cover use cases where people are forced to base the inheritance on enums such as ENUM('1','2','3')..

    #[ORM\DiscriminatorColumn(name: 'type', type: 'string')]
    #[ORM\DiscriminatorMap([
        '1' => EnumFooEntity::class,
        '2' => EnumBarEntity::class,
    ])]
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

No branches or pull requests

1 participant