doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.9k stars 2.5k forks source link

2.17 - Doctrine\DBAL\ArrayParameters\Exception\MissingPositionalParameter: Positional parameter at index 1 does not have a bound value. #11154

Open Gemineye opened 8 months ago

Gemineye commented 8 months ago

Bug Report

Q A
BC Break yes
Version 2.17.2

Summary

We have a ManyToOne Relation to an entity with a composite primary key. Since Version 2.17 with fetch="EAGER" we get the error:

Doctrine\DBAL\ArrayParameters\Exception\MissingPositionalParameter: Positional parameter at index 1 does not have a bound value.

Current behavior

Doctrine\DBAL\ArrayParameters\Exception\MissingPositionalParameter:
Positional parameter at index 1 does not have a bound value.

  at vendor/doctrine/dbal/src/ArrayParameters/Exception/MissingPositionalParameter.php:19
  at Doctrine\DBAL\ArrayParameters\Exception\MissingPositionalParameter::new(1)
     (vendor/doctrine/dbal/src/ExpandArrayParameters.php:50)
  at Doctrine\DBAL\ExpandArrayParameters->acceptPositionalParameter('?')
     (vendor/doctrine/dbal/src/SQL/Parser.php:88)
  at Doctrine\DBAL\SQL\Parser::Doctrine\DBAL\SQL\{closure}('?')
     (vendor/doctrine/dbal/src/SQL/Parser.php:102)
  at Doctrine\DBAL\SQL\Parser->parse('SELECT t0.id AS id_1, t0.description AS description_2, t0.sorting AS sorting_3, t0.changed_by AS changed_by_4, t0.created_at AS created_at_5, t0.updated_at AS updated_at_6, t0.service_id AS service_id_7, t0.company_id AS company_id_8, t0.company_id AS company_id_9 FROM services_description t0 WHERE t0.service_id IN (?) AND t0.company_id IN (?)', object(ExpandArrayParameters))
     (vendor/doctrine/dbal/src/Connection.php:1900)
  at Doctrine\DBAL\Connection->expandArrayParameters('SELECT t0.id AS id_1, t0.description AS description_2, t0.sorting AS sorting_3, t0.changed_by AS changed_by_4, t0.created_at AS created_at_5, t0.updated_at AS updated_at_6, t0.service_id AS service_id_7, t0.company_id AS company_id_8, t0.company_id AS company_id_9 FROM services_description t0 WHERE t0.service_id IN (?) AND t0.company_id IN (?)', array(array('service1', '12345', 'service2', '12345', 'service3', '12345', 'service4', '12345', 'service5', '12345', 'service6', '12345')), array(102, 102))
     (vendor/doctrine/dbal/src/Connection.php:1091)
  at Doctrine\DBAL\Connection->executeQuery('SELECT t0.id AS id_1, t0.description AS description_2, t0.sorting AS sorting_3, t0.changed_by AS changed_by_4, t0.created_at AS created_at_5, t0.updated_at AS updated_at_6, t0.service_id AS service_id_7, t0.company_id AS company_id_8, t0.company_id AS company_id_9 FROM dav_pharmacy_services_description t0 WHERE t0.service_id IN (?) AND t0.company_id IN (?)', array(array('service1', '12345', 'service2', '12345', 'service3', '12345', 'service4', '12345', 'service5', '12345', 'service6', '12345')), array(102, 102))
     (vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:939)
  at Doctrine\ORM\Persisters\Entity\BasicEntityPersister->loadAll(array('service' => array(object(Service), object(Service), object(Service), object(Service), object(Service), object(Service))))
     (vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:3182)
  at Doctrine\ORM\UnitOfWork->eagerLoadCollections(array('service1 12345' => object(PersistentCollection), 'service2 12345' => object(PersistentCollection), 'service3 12345' => object(PersistentCollection), 'service4 12345' => object(PersistentCollection), 'service5 12345' => object(PersistentCollection), 'service6 12345' => object(PersistentCollection)), array('fieldName' => 'serviceDescriptions', 'mappedBy' => 'service', 'targetEntity' => 'App\\SelfService\\Domain\\Model\\ServiceDescription', 'cascade' => array(), 'orphanRemoval' => false, 'fetch' => 3, 'type' => 4, 'inversedBy' => null, 'isOwningSide' => false, 'sourceEntity' => 'App\\SelfService\\Domain\\Model\\Service', 'isCascadeRemove' => false, 'isCascadePersist' => false, 'isCascadeRefresh' => false, 'isCascadeMerge' => false, 'isCascadeDetach' => false))
     (vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:3156)
  at Doctrine\ORM\UnitOfWork->triggerEagerLoads()
     (vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php:68)
  at Doctrine\ORM\Internal\Hydration\SimpleObjectHydrator->hydrateAllData()
     (vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php:270)
  at Doctrine\ORM\Internal\Hydration\AbstractHydrator->hydrateAll(object(Result), object(ResultSetMapping), array('deferEagerLoad' => true))
     (vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:943)
  at Doctrine\ORM\Persisters\Entity\BasicEntityPersister->loadAll(array('company' => '12345', 'online' => true), array('sorting' => 'ASC'), null, null)
     (vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php:225)
  at Doctrine\ORM\EntityRepository->findBy(array('company' => '12345', 'online' => true), array('sorting' => 'ASC'))
     (src/SelfService/Domain/Model/ServiceRepository.php:64)
  at App\SelfService\Domain\Model\ServiceRepository->findOfferedBy(object(ApoId), true)
     (src/Services/Controller/ServicesController.php:50)
  at App\Services\Controller\ServicesController->indexAction(object(ApoId), object(DataCollectorTranslator), object(Registry))
     (vendor/symfony/http-kernel/HttpKernel.php:163)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:75)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:202)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35)
  at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
     (vendor/autoload_runtime.php:40)
  at require_once('/var/www/app/vendor/autoload_runtime.php')
     (public/index.php:7)                

How to reproduce

Relation definition:

    /**
     * @ORM\Id
     * @ORM\Column(name="service_name")
     */
    protected string $serviceName;

    /**
     * @ORM\Id
     * @ORM\ManyToOne(targetEntity="App\Entity\Company")
     * @ORM\JoinColumn(name="company_id", referencedColumnName="company_id", nullable=false)
     */
    protected Company $company;

    /**
     * @var Collection<int, ServiceDescription>
     * @ORM\OneToMany(targetEntity="App\Entity\ServiceDescription", mappedBy="service")
     */
    protected Collection $serviceDescriptions;
    /**
     * @ORM\ManyToOne(targetEntity="App\Entity\Service", inversedBy="serviceDescriptions")
     * @ORM\JoinColumns(
     *     @ORM\JoinColumn(name="service_name", referencedColumnName="service_name"),
     *     @ORM\JoinColumn(name="company_id", referencedColumnName="company_id", nullable=false)
     * )
     */
    protected PharmacyService $serviceId;

Expected behavior

No Exception should occur.

themasch commented 6 months ago

Can confirm, we have the same issue here with >=2.17. Removing the fetch="EAGER" from a ManyToMany with a composite key on both sides (yeah, i know, old code) prevents the exception, but tanks performance. Currently working around this by locking the doctrine/orm version to ^2.0,<2.17.0, which works for now, but is ofc not desirable.

derrabus commented 6 months ago

Can either of you try to provide a failing functional test or maybe even investigate the issue?

themasch commented 6 months ago

@derrabus I did some debugging, switching between 2.16.x and 2.18.x and re-running the failing test cases, trying to find the point where they diverge. I am not sure if thats the correct path, since changes seem to be quite fundamental around eager loading of collections at that point.

For me, it looks like with 2.16.x eager fetching of these collections does not really trigger at all in the few test cases that break with 2.17.

With 2.17, one of our test fails when doing $repo->findAll() of an entity that has a OneToMany with fetch=EAGER, the others fail when calling refresh on such an entity.

I think this here should looks suspicious to me:

We have an entity LineItem with a composite key, lets call them order_id and position. There is another entity LineItemError which is joined (OneToMany) via order_id and position . order_id is an integer, position is a string.

When doing a $repo->findAll() we go from EntityRepository::findAll to ::findBy to BasicEntityPersister::loadAll, AbstractHydrator::hydrateAll , SimpleObjectHydrator::hydrateAllData, UnitOfWork::triggerEagerLoad, UnitOfWork::eagerLoadCollections, and back at BasicEntityPersister::loadAll (this time for the collection.

The SQL it generate here looks something like this:

SELECT [...],, t0.position AS position_2, t0.order_id AS order_id_3, [...], t0.order_id AS order_id_8, t0.position AS position_9 FROM [...]line_item_error t0 WHERE t0.order_id IN (?) AND t0.position IN (?)

Which... I am not entirely sure about. Maybe its getting late, but that looks like it could find the wrong entries. Maybe we set something up incorrectly? Or it may fix it in a later step?

The $criteria has an array { "lineItem": array<LineItem> } with 3 LineItems (or, in this case, for some reason, two proxies and one "real" LineItem. Guess thats related to things the test does).

From that expandParameters collects this $params: [ 0 => [ 1, '1', 1, '2', 1, '3' ] ]

So for me, that looks like it mixes both keys, interweaving them. [ $order_id[0] , $position[0], $order_id[1], $position[1], ... ]. Not sure if thats supposed to happen, with that query I would have assumed a result that looks more like [0 => [1, 1, 1], 1 => ['1', '2', '3'] ].

$types has [101, 102]

grafik (ignore the "valueName", thats phpstorm being confused)

When then going into executeQuery the SQL\Parser tries to patch in the parameters and cannot find values for the second parameter (since we only gave it one list of values, not two).

Soooo either we f'ed up big time (more than we know!) and setup the OneToMany join totally incorrect, without noticing it for a very long time, or something about the new changes since 2.17.0 breaks our broken use-case.

I am aware that what we do here is not exactly "best practice". But in my defense: it kind up worked up until know ;)

I hope to find some more time tomorrow and come up with a minimal reproducer.

derrabus commented 6 months ago

Can you create a functional test that reproduces your scenario?

themasch commented 6 months ago

I'll give it a try to build a functional test here that triggers this. If that does not work out I already have a "minmal" reproducer with ~160 lines of php (including doctrine bootstrap) that reproduces the issue just fine. I can push/share that if it helps.

But will try to build a functional test first, that would be the best cast.

themasch commented 6 months ago

@derrabus I created https://github.com/doctrine/orm/pull/11289 At least locally for me that triggers the exception. Please let me know if you need something more, or if I did something wrong with this.

derrabus commented 6 months ago

Thank you. Your test is failing although it's a different error message than the one you've described.

themasch commented 6 months ago

The error message seem to depend on the php version (not sure why it would): on <=7.2 we get a generic DBAL\Exception "Undefined offset: 2" (https://github.com/doctrine/orm/actions/runs/8002637962/job/21857153118?pr=11289)

on >=7.3 we get the "expected": Doctrine\DBAL\ArrayParameters\Exception\MissingPositionalParameter: Positional parameter at index 1 does not have a bound value. ( https://github.com/doctrine/orm/actions/runs/8002637962/job/21857153428?pr=11289 )

Same on 8.3: https://github.com/doctrine/orm/actions/runs/8002637962/job/21857155103?pr=11289

derrabus commented 6 months ago

That's probably due to DBAL 2 vs. DBAL 3. 🙂

andrzej-orbitvu commented 6 months ago

I have the same error in 2.18.1 with PHP 8.3.3 for OneToMany

class Asset
{
    #[Id]
    #[Column(type: 'bigint', options: ['unsigned' => true])]
    private int $assetId;

    #[Id]
    #[Column(type: 'bigint', options: ['unsigned' => true])]
    private int $productId;

    #[OneToMany(mappedBy: 'asset', targetEntity: AssetContent::class, cascade: ["persist", "remove"], fetch: "EAGER")]
    private Collection $content;
}

class AssetContent
{
    // .. 

    #[ManyToOne(targetEntity: Asset::class, inversedBy: 'content')]
    #[JoinColumn(name:"asset_id", referencedColumnName:"asset_id", nullable: false, onDelete: 'CASCADE')]
    #[JoinColumn(name:"product_id", referencedColumnName:"product_id", nullable: false, onDelete: 'CASCADE')]
    private Asset $asset;
}

when I call

$em->find(Asset::class, ['assetId' => $assetId, 'productId' => $productId]);
themasch commented 5 months ago

So, I played around a bit more, and debugged a few scenarios.

First, no surprise, but the same issue triggers if you manually try to load the associated entities: (using the test case in https://github.com/doctrine/orm/pull/11289 as base for the example here)

        // remove the fetch="EAGER" from RootEntity::$secondLevel before this
        $x = $this->_em->getRepository(RootEntity::class)->findAll()[0];
        $r2 = $this->_em->getRepository(SecondLevel::class)->findBy(['root' => [$x]]);

This is because this does exactly the same as Doctrine itself would do on a EAGER fetch.

So, I think (and maybe I am late in understanding this and stating the obvious) the root problem here is, that the query doctrine generate here to load the association does not support fetching the associated entities for more than one owning entity. Meaing ->findBy(['root' => $x]) works, but ->findBy(['root' => [$x]]) (which is what eagerLoadCollections calls) does not.

The query that generates looks like this:

SELECT t0.id AS id_1, t0.upperId AS upperId_2, t0.other_key AS other_key_3, t0.other_key AS other_key_4, t0.upper_id AS upper_id_5 FROM eager_composite_join_second_level t0 WHERE t0.other_key IN (?) AND t0.upper_id IN (?)

Which will never work. Even if expandParameters -> getValues / getIndividualValue would split the two used keys up in two params (which it does not), the result would be unsound. I think it could potentially return incorrect results:

a IN (1, 2) AND b in (3, 4) is not the same as (a, b) IN ((1, 3), (2, 4)), right? But I am not sure if the latter is valid in all (or even any) SQL dialect.

A naive sledgehammer approach of "fixing" this would be, to, in eagerLoadCollections fall back to loadOneToManyCollection / loadManyToManyCollection if $targetEntity isIdentifierComposite ? But I do not claim to understand even the basics of doctrine internals.

Just putting that here in case it helps anyone.

Moral of the story: do not use composite identifiers / join columns ;)

themasch commented 5 months ago

With the merge of #11289 an the release of 2.19.2 I think this bug is fixed (at least in our test suite) and this ticket can be closed?

Thanks a lot @derrabus and @beberlei

Gemineye commented 5 months ago

@greg0ire The update does not seem to fix our error :(

/**
 * @var Collection<int, PharmacyServiceDescription>
 * @ORM\OneToMany(targetEntity="MeinDAV\SelfService\Domain\Model\PharmacyServiceDescription", mappedBy="serviceId", fetch="EAGER")
 */
protected Collection $serviceDescriptions;

Without "fetch="EAGER"" it works.