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

After update from version 2.20.0 to 2.20.1 the Hydratation generate unexpected Warning #11783

Open
tomasjanicek opened this issue Jan 7, 2025 · 10 comments

Comments

@tomasjanicek
Copy link

tomasjanicek commented Jan 7, 2025

Bug Report

Q A
Version 2.20.1
Previous Version if the bug is a regression 2.20.0

Summary

After updating to the latest version, hydration is not working properly. It generates a warning that ends up throwing an exception in the application.

Current behavior

The application runs on symfony 6.4 LTS. and PHP 8.4
we have ManyToMany connection between entitites...

class Lector {
   ...
    #[ORM\JoinTable(name: 'lector_domains')]
    #[ORM\ManyToMany(targetEntity: Domain::class, indexBy: 'id')]
    protected Collection $domains;
}

composer doctrine depedencies:

        "doctrine/dbal": "^3.5",
        "doctrine/doctrine-bundle": "^2.7",
        "doctrine/doctrine-fixtures-bundle": "^3.4",
        "doctrine/doctrine-migrations-bundle": "^3.2",
        "doctrine/mongodb-odm": "^2.4",
        "doctrine/orm": "2.20.1",

Stack trace:

ErrorException:
Warning: Undefined array key "id_1"

  at vendor/doctrine/orm/src/Internal/Hydration/ObjectHydrator.php:509
  at Doctrine\ORM\Internal\Hydration\ObjectHydrator->hydrateRowData()
     (vendor/doctrine/orm/src/Internal/Hydration/ObjectHydrator.php:149)
  at Doctrine\ORM\Internal\Hydration\ObjectHydrator->hydrateAllData()
     (vendor/doctrine/orm/src/Internal/Hydration/AbstractHydrator.php:272)
  at Doctrine\ORM\Internal\Hydration\AbstractHydrator->hydrateAll()
     (vendor/doctrine/orm/src/Persisters/Entity/BasicEntityPersister.php:1034)
  at Doctrine\ORM\Persisters\Entity\BasicEntityPersister->loadCollectionFromStatement()
     (vendor/doctrine/orm/src/Persisters/Entity/BasicEntityPersister.php:1044)
  at Doctrine\ORM\Persisters\Entity\BasicEntityPersister->loadManyToManyCollection()
     (vendor/doctrine/orm/src/UnitOfWork.php:3305)
  at Doctrine\ORM\UnitOfWork->loadCollection()
     (vendor/doctrine/orm/src/PersistentCollection.php:706)
  at Doctrine\ORM\PersistentCollection->doInitialize()
     (vendor/doctrine/orm/src/PersistentCollection.php:218)
  at Doctrine\ORM\PersistentCollection->initialize()
     (vendor/doctrine/collections/src/AbstractLazyCollection.php:157)
  at Doctrine\Common\Collections\AbstractLazyCollection->toArray()
     (src/Seduo/Core/Entity/WithDomainsTrait.php:22)
  at Seduo\Core\Entity\Lector->getDomains()
     (src/Seduo/Core/Entity/Course.php:445)
  at Seduo\Core\Entity\Course::{closure:Seduo\Core\Entity\Course::getLectors():444}()
  at array_filter()
     (vendor/doctrine/collections/src/ArrayCollection.php:381)
  at Doctrine\Common\Collections\ArrayCollection->filter()
     (vendor/doctrine/collections/src/AbstractLazyCollection.php:239)
  at Doctrine\Common\Collections\AbstractLazyCollection->filter()
     (src/Seduo/Core/Entity/Course.php:444)
  at Seduo\Core\Entity\Course->getLectors()
     (src/Seduo/Core/Entity/Course.php:457)
  at Seduo\Core\Entity\Course->hasLectors()
     (src/Seduo/AdminBundle/Grid/Courses/Builder/CourseRowBuilder.php:47)
  at Seduo\AdminBundle\Grid\Courses\Builder\CourseRowBuilder->buildFromObject()
     (src/Seduo/Grid/Factory/GridFactory.php:51)
  at Seduo\Grid\Factory\GridFactory->createGrid()
     (src/Seduo/AdminBundle/Grid/Courses/CoursesGridFactory.php:53)
  at Seduo\AdminBundle\Grid\Courses\CoursesGridFactory->create()
     (src/Seduo/AdminBundle/Controller/CoursesController.php:62)
  at Seduo\AdminBundle\Controller\CoursesController->filterableList()
     (vendor/symfony/http-kernel/HttpKernel.php:181)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw()
     (vendor/symfony/http-kernel/HttpKernel.php:76)
  at Symfony\Component\HttpKernel\HttpKernel->handle()
     (vendor/symfony/http-kernel/Kernel.php:197)
  at Symfony\Component\HttpKernel\Kernel->handle()
     (vendor/symfony/http-kernel/HttpCache/SubRequestHandler.php:86)
  at Symfony\Component\HttpKernel\HttpCache\SubRequestHandler::handle()
     (vendor/symfony/http-kernel/HttpCache/HttpCache.php:476)
  at Symfony\Component\HttpKernel\HttpCache\HttpCache->forward()
     (vendor/symfony/framework-bundle/HttpCache/HttpCache.php:68)
  at Symfony\Bundle\FrameworkBundle\HttpCache\HttpCache->forward()
     (vendor/symfony/http-kernel/HttpCache/HttpCache.php:451)
  at Symfony\Component\HttpKernel\HttpCache\HttpCache->fetch()
     (vendor/symfony/http-kernel/HttpCache/HttpCache.php:349)
  at Symfony\Component\HttpKernel\HttpCache\HttpCache->lookup()
     (vendor/symfony/http-kernel/HttpCache/HttpCache.php:222)
  at Symfony\Component\HttpKernel\HttpCache\HttpCache->handle()
     (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35)
  at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
     (vendor/autoload_runtime.php:30)
  at require_once('/srv/www/seduo/vendor/autoload_runtime.php')
     (web/app_dev.php:7)                

Expected behavior

does not generate a warning..

How to reproduce

for us it was reflected in the fact that we updated from version 2.20.0 to 2.20.1

@greg0ire
Copy link
Member

greg0ire commented Jan 7, 2025

Please always share stack traces as plain text rather than images, as indicated in https://symfony.com/doc/current/contributing/code/stack_trace.html#stack-traces-in-your-web-browser

Here is the code comparison: 2.20.0...2.20.1

You could try narrowing it down by following https://dev.to/greg0ire/bisecting-vendors-12kd

@mcapinha-mxp
Copy link

Hello,

We've just noticed this same error as well.
Reverting to 2.20.0 fixes it for us. I traced it back to this commit in 2.20.1: 7d1b24f#diff-2cbf87f941a104db9881228f86e16e3c9f894a88c7ac9245b0632de36b8b887cR2476
If I revert that, 2.20.1 works for us.

@greg0ire
Copy link
Member

greg0ire commented Jan 7, 2025

Great job! Ping @goetas , can you please take a look?

@greg0ire
Copy link
Member

greg0ire commented Jan 7, 2025

For anybody experiencing this, what would help would be:

  • a plain text stack trace
  • a PR with a PHPUnit test case reproduce the issue

@goetas
Copy link
Member

goetas commented Jan 7, 2025

I'm skeptical that the stacktrace posted in the screenshot and the commit are related. the changes in the commit affect the refresh() method, that it is never invoked during hydration. it is invoked only if you explicitly refresh an entity using the entity manager.

I also have tried to reproduce it with a few many to many collections and indexby options, but all of them worked as expected.
it would be great to have a way to reproduce the issue so i can debug it

@bobvandevijver
Copy link
Contributor

bobvandevijver commented Jan 7, 2025

I have the same issue, but can confirm that it is not related to the commit linked before. I have traced it back to #11694: reverting the sources changes from there fixes the issue.

Warning: Undefined array key "locale_11"

in vendor/doctrine/orm/src/Internal/Hydration/ObjectHydrator.php (line 509)
in vendor/doctrine/orm/src/Internal/Hydration/ObjectHydrator.php -> hydrateRowData (line 149)
in vendor/doctrine/orm/src/Internal/Hydration/AbstractHydrator.php -> hydrateAllData (line 272)
in vendor/doctrine/orm/src/Persisters/Entity/BasicEntityPersister.php -> hydrateAll (line 1034)
in vendor/doctrine/orm/src/Persisters/Entity/BasicEntityPersister.php -> loadCollectionFromStatement (line 1863)
in vendor/doctrine/orm/src/UnitOfWork.php -> loadOneToManyCollection (line 3301)
in vendor/doctrine/orm/src/UnitOfWork.php -> loadCollection (line 3169)
in vendor/doctrine/orm/src/Internal/Hydration/SimpleObjectHydrator.php -> createEntity (line 173)
in vendor/doctrine/orm/src/Internal/Hydration/SimpleObjectHydrator.php -> hydrateRowData (line 65)
in vendor/doctrine/orm/src/Internal/Hydration/AbstractHydrator.php -> hydrateAllData (line 272)
in vendor/doctrine/orm/src/Persisters/Entity/BasicEntityPersister.php -> hydrateAll (line 976)
in vendor/doctrine/orm/src/EntityRepository.php -> loadAll (line 224)

What I can see from a debug run is that the $this->currentPersisterContext->selectColumnListSql is returned when the additional condition is not there, which is now skipped because the filter hash has changed due to a parameter change done in the application.

What's interesting to see is that the aliases in the query do not start with _1:

SELECT
  t0.name AS name_13,
  t0.terms_and_conditions AS terms_and_conditions_14,
  t0.utwente_code_of_conduct AS utwente_code_of_conduct_15,
  t0.pickup_service_details AS pickup_service_details_16,
  t0.dogroup_parent_terms_and_conditions AS dogroup_parent_terms_and_conditions_17,
  t0.mentor_terms_and_conditions AS mentor_terms_and_conditions_18,
  t0.dogroupParentInfo AS dogroupParentInfo_19,
  t0.mentorInfo AS mentorInfo_20,
  t0.volunteerInfo AS volunteerInfo_21,
  t0.id AS id_22,
  t0.locale AS locale_23,
  t0.translatable_id AS translatable_id_24
FROM
  EventTranslation t0
WHERE
  t0.translatable_id = ?

While the same query, executed earlier in the same request, does start with _1 as alias for name:

SELECT
  t0.name AS name_1,
  t0.terms_and_conditions AS terms_and_conditions_2,
  t0.utwente_code_of_conduct AS utwente_code_of_conduct_3,
  t0.pickup_service_details AS pickup_service_details_4,
  t0.dogroup_parent_terms_and_conditions AS dogroup_parent_terms_and_conditions_5,
  t0.mentor_terms_and_conditions AS mentor_terms_and_conditions_6,
  t0.dogroupParentInfo AS dogroupParentInfo_7,
  t0.mentorInfo AS mentorInfo_8,
  t0.volunteerInfo AS volunteerInfo_9,
  t0.id AS id_10,
  t0.locale AS locale_11,
  t0.translatable_id AS translatable_id_12
FROM
  EventTranslation t0
WHERE
  t0.translatable_id = ?

That is where the error comes from, which is Undefined array key "locale_11" in my case.

Resetting the alias counter when not returning the cached column sql works for me. See f5e0d89 for the changeset. It's quite late already, so I'm unable to create a full PR with tests at this time, and I'm also not sure this is the correct fix or that the fix should be done somewhere else. Maybe someone with more understanding of the internals can my research results to make a complete fix 😄

@tomasjanicek
Copy link
Author

tomasjanicek commented Jan 8, 2025

Please always share stack traces as plain text rather than images, as indicated in https://symfony.com/doc/current/contributing/code/stack_trace.html#stack-traces-in-your-web-browser

Here is the code comparison: 2.20.0...2.20.1

You could try narrowing it down by following https://dev.to/greg0ire/bisecting-vendors-12kd

sorry for that.. I change image to stack trace in issue definition.

@greg0ire
Copy link
Member

greg0ire commented Jan 8, 2025

Cc @dbannik

@mcapinha-mxp
Copy link

mcapinha-mxp commented Jan 8, 2025

I'm skeptical that the stacktrace posted in the screenshot and the commit are related. the changes in the commit affect the refresh() method, that it is never invoked during hydration. it is invoked only if you explicitly refresh an entity using the entity manager.

I also have tried to reproduce it with a few many to many collections and indexby options, but all of them worked as expected. it would be great to have a way to reproduce the issue so i can debug it

Sorry @goetas , my bad. The issue we're having has the same error message but it's not coming from hydrate, it's coming from refresh() and reverting that commit does indeed fix it for us.
I'll create a separate issue for that, my mistake thinking this issue was the same as ours. Sorry once again.

@bobvandevijver
Copy link
Contributor

The issue is triggered by having an index column: the result set mapping is setting the index alias as the first found column name here:

public function addIndexBy($alias, $fieldName)
{
$found = false;
foreach (array_merge($this->metaMappings, $this->fieldMappings) as $columnName => $columnFieldName) {
if (! ($columnFieldName === $fieldName && $this->columnOwnerMap[$columnName] === $alias)) {
continue;
}
$this->addIndexByColumn($alias, $columnName);
$found = true;
break;
}

However, with the select query being regenerated, the result set mapping actually has two aliases for the same field, and the one that is selected in the addIndexBy and used in the hydrateRowData is then referring to a column that isn't in the select statement:

if (isset($this->resultSetMapping()->indexByMap[$dqlAlias])) {
$resultKey = $row[$this->resultSetMapping()->indexByMap[$dqlAlias]];

I think the issue is that each time the getSQLColumnAlias is called it generates a new alias for the column even when there already was one generated:

public function getSQLColumnAlias($columnName)
{
return $this->quoteStrategy->getColumnAlias($columnName, $this->currentPersisterContext->sqlAliasCounter++, $this->platform);
}

However, in the result set mapping that alias can either be stored as a field or meta result, so just looking into the field result map to return the already existing alias is probably a no go. That would mean that either the method using the getSQLColumnAlias method need to check the result set mapping for a usable alias first, or that the getSQLColumnAlias needs to be passed context about how the alias is to be used, so it can check the mapping itself.

I've added a failing test to show the issue: #11786

bobvandevijver added a commit to bobvandevijver/doctrine-orm that referenced this issue Jan 8, 2025
bobvandevijver added a commit to bobvandevijver/doctrine-orm that referenced this issue Jan 8, 2025
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

5 participants