Skip to content

Loading…

DDC-2674: Hydrate not working correctly #3412

Closed
doctrinebot opened this Issue · 7 comments

2 participants

@doctrinebot

Jira issue originally created by user flip101:

Expected behaviour: get 5 countries in africa (and other continents with countries)
Actual behaviour: getting 1 country in africa (and other continents with countries)

// Schema:
One Continent to Many Country
(Continent is called Region in the schema)

// DQL

$query = $em->createQuery("
    SELECT continent, country
    FROM Geo:Continent continent
    JOIN continent.countries country
    ORDER BY continent.order ASC, country.name ASC
");

// Filter

<?php
namespace MyVendor\MyBundle\DoctrineFilter;

use Doctrine\ORM\Mapping\ClassMetaData,
    Doctrine\ORM\Query\Filter\SQLFilter;

class VioCountryFilter extends SQLFilter {

    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) {
        if ($targetEntity->name === 'MyVendor\Entity\Geo\Country') {
            return $targetTableAlias . '.isVioLand = 1';
        } else {
            return '';
        }
    }
}

// Filter config (in Symfony 2.3) YAML

doctrine:
    orm:
        entity_managers:
            default:
                filters:
                    vio_country:
                        class: MyVendor\MyBundle\DoctrineFilter\VioCountryFilter
                        enabled: true

// Generated - SQL

SELECT 
  r0_.id AS id0, 
  r0_.Region AS Region1, 
  r0_.Sort AS Sort2, 
  l1_.LandId AS LandId3, 
  l1_.IsVioLand AS IsVioLand4, 
  l1_.Max AS Max22, 
  l1_.Min AS Min23, 
  l1_.LandNaam AS LandNaam24, 
  l1_.Population AS Population29, 
  l1_.RegionId AS RegionId35 
FROM 
  Region r0_ WITH (NOLOCK) 
  INNER JOIN Land l1* ON r0_.id = l1*.RegionId 
  AND (l1_.isVioLand = 1) 
ORDER BY 
  r0_.Sort ASC, 
  l1_.LandNaam ASC

// Executed SQL in Management Studio:
This is only displaying the 6th continent: Africa (do not show other continents)
(Sort goes from 1 to 6)

||id0||Region1||Sort2||LandId3||IsVioLand4||Max22||Min23||LandNaam24||Population29||RegionId35||
|1|Africa|6|5|1|24.0|13.0|Algeria|37762962|1|
|1|Africa|6|6|1|27.0|17.0|Egypt|79392466|1|
|1|Africa|6|10|1|23.0|13.0|Morocco|32059424|1|
|1|Africa|6|14|1|22.0|12.0|South Africa|50586757|1|
|1|Africa|6|13|1|24.0|15.0|Tunisia|10673800|1|

HAS 5 COUNTRIES IN AFRICA !!!

// Dump = The actual behaviour

$continents = $query->getResult();
var_dump($continents[5]);

(also made this dump shorter)

class MyVendor\Entity\Geo\Continent#1290 (4) {
  protected $id =>
  int(1)
  protected $countries =>
  class Doctrine\ORM\PersistentCollection#1292 (9) {
    private $snapshot =>
    array(1) { //               <-- HAS 1 COUNTRIES IN AFRICA !!!
      [0] =>
      class MyVendor\Entity\Geo\Country#1293 (38) {
        ...
      }
    }
    private $coll =>
    class Doctrine\Common\Collections\ArrayCollection#1291 (1) {
      private $_elements =>
      array(1) { //                 <-- HAS 1 COUNTRIES IN AFRICA !!!
        ...
      }
    }
  }
  protected $name =>
  string(6) "Africa"
  protected $order =>
  int(6)
}
@doctrinebot

Comment created by flip101:

I also don't get the right results when turning the filter off.

Also i thought maybe the hydrator was tripping over column names like "Min", "Max" or "Population". But after renaming those i still get the same results.

@doctrinebot

Comment created by flip101:

I guess it had to be the ObjectHydrater .. just on intuition i started to place debug points in the hydrateRowData() method. I can definitely see a different path of execution from the countries that are included from the ones that are not. So i guess it has to be somewhere in there. But the logic is not really easy to understand.

    protected function hydrateRowData(array $row, array &$cache, array &$result)
    {
if (isset($row['Region1']) AND $row['Region1'] === 'Africa') {
    $DEBUG = true;
} else {
    $DEBUG = false;
}
        // Initialize
        $id = $this->idTemplate; // initialize the id-memory
        $nonemptyComponents = array();
        // Split the row data into chunks of class data.
        $rowData = $this->gatherRowData($row, $cache, $id, $nonemptyComponents);

        // Extract scalar values. They're appended at the end.
        if (isset($rowData['scalars'])) {
            $scalars = $rowData['scalars'];

            unset($rowData['scalars']);

            if (empty($rowData)) {
                <ins></ins>$this->resultCounter;
            }
        }

        // Extract "new" object constructor arguments. They're appended at the end.
        if (isset($rowData['newObjects'])) {
            $newObjects = $rowData['newObjects'];

            unset($rowData['newObjects']);

            if (empty($rowData)) {
                <ins></ins>$this->resultCounter;
            }
        }

        // Hydrate the data chunks
        foreach ($rowData as $dqlAlias => $data) {
if ($DEBUG) echo "\n\nEntity: ". $dqlAlias. "\n";
if ($DEBUG) echo "name: {$data['name']} \n";
            $entityName = $this->_rsm->aliasMap[$dqlAlias];

            if (isset($this->_rsm->parentAliasMap[$dqlAlias])) {
                // It's a joined result

                $parentAlias = $this->_rsm->parentAliasMap[$dqlAlias];
                // we need the $path to save into the identifier map which entities were already
                // seen for this parent-child relationship
                $path = $parentAlias . '.' . $dqlAlias;

                // We have a RIGHT JOIN result here. Doctrine cannot hydrate RIGHT JOIN Object-Graphs
                if ( ! isset($nonemptyComponents[$parentAlias])) {
                    // TODO: Add special case code where we hydrate the right join objects into identity map at least
                    continue;
                }

                // Get a reference to the parent object to which the joined element belongs.
                if ($this->_rsm->isMixed && isset($this->rootAliases[$parentAlias])) {
                    $first = reset($this->resultPointers);
                    $parentObject = $first[key($first)];
                } else if (isset($this->resultPointers[$parentAlias])) {
                    $parentObject = $this->resultPointers[$parentAlias];
                } else {
                    // Parent object of relation not found, so skip it.
                    continue;
                }

                $parentClass    = $this->ce[$this->_rsm->aliasMap[$parentAlias]];
                $oid            = spl*object*hash($parentObject);
                $relationField  = $this->_rsm->relationMap[$dqlAlias];
                $relation       = $parentClass->associationMappings[$relationField];
                $reflField      = $parentClass->reflFields[$relationField];

                // Check the type of the relation (many or single-valued)
                if ( ! ($relation['type'] & ClassMetadata::TO_ONE)) {
if ($DEBUG) echo "PATH A\n";
                    $reflFieldValue = $reflField->getValue($parentObject);
                    // PATH A: Collection-valued association
                    if (isset($nonemptyComponents[$dqlAlias])) {
if ($DEBUG) echo "1\n";
                        $collKey = $oid . $relationField;
                        if (isset($this->initializedCollections[$collKey])) {
if ($DEBUG) echo "1 1\n";
                            $reflFieldValue = $this->initializedCollections[$collKey];
                        } else if ( ! isset($this->existingCollections[$collKey])) {
if ($DEBUG) echo "1 2\n";
                            $reflFieldValue = $this->initRelatedCollection($parentObject, $parentClass, $relationField, $parentAlias);
                        }

                        $indexExists    = isset($this->identifierMap[$path][$id[$parentAlias]][$id[$dqlAlias]]);
                        $index          = $indexExists ? $this->identifierMap[$path][$id[$parentAlias]][$id[$dqlAlias]] : false;
                        $indexIsValid   = $index !== false ? isset($reflFieldValue[$index]) : false;

                        if ( ! $indexExists || ! $indexIsValid) {
if ($DEBUG) echo "1 3\n";
                            if (isset($this->existingCollections[$collKey])) {
if ($DEBUG) echo "1 3 1\n";
if ($DEBUG AND isset($data['name']) AND $data['name'] === 'Algeria') {
    echo 'I could var_dump() some useful info here .. but what ?';
}
                                // Collection exists, only look for the element in the identity map.
                                if ($element = $this->getEntityFromIdentityMap($entityName, $data)) {
                                    $this->resultPointers[$dqlAlias] = $element;
                                } else {
                                    unset($this->resultPointers[$dqlAlias]);
                                }
                            } else {
if ($DEBUG) echo "1 3 2\n";
                                $element = $this->getEntity($data, $dqlAlias);

                                if (isset($this->_rsm->indexByMap[$dqlAlias])) {
if ($DEBUG) echo "1 3 2 1\n";
                                    $indexValue = $row[$this->_rsm->indexByMap[$dqlAlias]];
                                    $reflFieldValue->hydrateSet($indexValue, $element);
                                    $this->identifierMap[$path][$id[$parentAlias]][$id[$dqlAlias]] = $indexValue;
                                } else {
if ($DEBUG) echo "1 3 2 2\n";
                                    $reflFieldValue->hydrateAdd($element);
                                    $reflFieldValue->last();
                                    $this->identifierMap[$path][$id[$parentAlias]][$id[$dqlAlias]] = $reflFieldValue->key();
                                }
                                // Update result pointer
                                $this->resultPointers[$dqlAlias] = $element;
                            }
                        } else {
if ($DEBUG) echo "1 4\n";
                            // Update result pointer
                            $this->resultPointers[$dqlAlias] = $reflFieldValue[$index];
                        }
                    } else if ( ! $reflFieldValue) {
if ($DEBUG) echo "2\n";
                        $reflFieldValue = $this->initRelatedCollection($parentObject, $parentClass, $relationField, $parentAlias);
                    } else if ($reflFieldValue instanceof PersistentCollection && $reflFieldValue->isInitialized() === false) {
if ($DEBUG) echo "3\n";
                        $reflFieldValue->setInitialized(true);
                    }

                } else {
if ($DEBUG) echo "PATH B\n";
                    // PATH B: Single-valued association
                    $reflFieldValue = $reflField->getValue($parentObject);
                    if ( ! $reflFieldValue || isset($this->*hints[Query::HINT_REFRESH]) || ($reflFieldValue instanceof Proxy && !$reflFieldValue->__isInitialized_*)) {
                        // we only need to take action if this value is null,
                        // we refresh the entity or its an unitialized proxy.
                        if (isset($nonemptyComponents[$dqlAlias])) {
                            $element = $this->getEntity($data, $dqlAlias);
                            $reflField->setValue($parentObject, $element);
                            $this->_uow->setOriginalEntityProperty($oid, $relationField, $element);
                            $targetClass = $this->ce[$relation['targetEntity']];

                            if ($relation['isOwningSide']) {
                                //TODO: Just check hints['fetched'] here?
                                // If there is an inverse mapping on the target class its bidirectional
                                if ($relation['inversedBy']) {
                                    $inverseAssoc = $targetClass->associationMappings[$relation['inversedBy']];
                                    if ($inverseAssoc['type'] & ClassMetadata::TO_ONE) {
                                        $targetClass->reflFields[$inverseAssoc['fieldName']]->setValue($element, $parentObject);
                                        $this->*uow->setOriginalEntityProperty(spl_object*hash($element), $inverseAssoc['fieldName'], $parentObject);
                                    }
                                } else if ($parentClass === $targetClass && $relation['mappedBy']) {
                                    // Special case: bi-directional self-referencing one-one on the same class
                                    $targetClass->reflFields[$relationField]->setValue($element, $parentObject);
                                }
                            } else {
                                // For sure bidirectional, as there is no inverse side in unidirectional mappings
                                $targetClass->reflFields[$relation['mappedBy']]->setValue($element, $parentObject);
                                $this->*uow->setOriginalEntityProperty(spl_object*hash($element), $relation['mappedBy'], $parentObject);
                            }
                            // Update result pointer
                            $this->resultPointers[$dqlAlias] = $element;
                        } else {
                            $this->_uow->setOriginalEntityProperty($oid, $relationField, null);
                            $reflField->setValue($parentObject, null);
                        }
                        // else leave $reflFieldValue null for single-valued associations
                    } else {
                        // Update result pointer
                        $this->resultPointers[$dqlAlias] = $reflFieldValue;
                    }
                }
            } else {
if ($DEBUG) echo "PATH C\n";
                // PATH C: Its a root result element
                $this->rootAliases[$dqlAlias] = true; // Mark as root alias
                $entityKey = $this->_rsm->entityMappings[$dqlAlias] ?: 0;

                // if this row has a NULL value for the root result id then make it a null result.
                if ( ! isset($nonemptyComponents[$dqlAlias]) ) {
                    if ($this->_rsm->isMixed) {
                        $result[] = array($entityKey => null);
                    } else {
                        $result[] = null;
                    }
                    $resultKey = $this->resultCounter;
                    <ins></ins>$this->resultCounter;
                    continue;
                }

                // check for existing result from the iterations before
                if ( ! isset($this->identifierMap[$dqlAlias][$id[$dqlAlias]])) {
                    $element = $this->getEntity($rowData[$dqlAlias], $dqlAlias);

                    if ($this->_rsm->isMixed) {
                        $element = array($entityKey => $element);
                    }

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

                        if (isset($this->_hints['collection'])) {
                            $this->_hints['collection']->hydrateSet($resultKey, $element);
                        }

                        $result[$resultKey] = $element;
                    } else {
                        $resultKey = $this->resultCounter;
                        <ins></ins>$this->resultCounter;

                        if (isset($this->_hints['collection'])) {
                            $this->_hints['collection']->hydrateAdd($element);
                        }

                        $result[] = $element;
                    }

                    $this->identifierMap[$dqlAlias][$id[$dqlAlias]] = $resultKey;

                    // Update result pointer
                    $this->resultPointers[$dqlAlias] = $element;

                } else {
                    // Update result pointer
                    $index = $this->identifierMap[$dqlAlias][$id[$dqlAlias]];
                    $this->resultPointers[$dqlAlias] = $result[$index];
                    $resultKey = $index;
                    /*if ($this->_rsm->isMixed) {
                        $result[] = $result[$index];
                        <ins></ins>$this->_resultCounter;
                    }*/
                }
            }
        }

        // Append scalar values to mixed result sets
        if (isset($scalars)) {
if ($DEBUG) echo "scalars";
            if ( ! isset($resultKey) ) {
                if (isset($this->_rsm->indexByMap['scalars'])) {
                    $resultKey = $row[$this->_rsm->indexByMap['scalars']];
                } else {
                    $resultKey = $this->resultCounter - 1;
                }
            }

            foreach ($scalars as $name => $value) {
                $result[$resultKey][$name] = $value;
            }
        }

        // Append new object to mixed result sets
        if (isset($newObjects)) {
if ($DEBUG) echo "newObjects";
            if ( ! isset($resultKey) ) {
                $resultKey = $this->resultCounter - 1;
            }

            $count = count($newObjects);

            foreach ($newObjects as $objIndex => $newObject) {
                $class  = $newObject['class'];
                $args   = $newObject['args'];
                $obj    = $class->newInstanceArgs($args);

                if ($count === 1) {
                    $result[$resultKey] = $obj;

                    continue;
                }

                $result[$resultKey][$objIndex] = $obj;
            }
        }
    }

Result for the first 3 rows:

Entity: continent
name: Africa 
PATH C


Entity: country
name: South Africa 
PATH A
1
1 2
1 3
1 3 2
1 3 2 2


Entity: continent
name: Africa 
PATH C


Entity: country
name: Algeria 
PATH A
1
1 2
1 3
1 3 1
I could var_dump() some useful info here .. but what ?

Entity: continent
name: Africa 
PATH C


Entity: country
name: Egypt 
PATH A
1
1 3
1 3 1
@doctrinebot

Comment created by flip101:

This goes wrong (shows 1 country in africa):

$cr = $this->get('continent_repository');

$licensedContinents = $cr->getAllowedContinentsWithCountries($this->getUser());
$continents = $cr->getContinentsWithCountries();
foreach ($continents[5]->getCountries()->getIterator() as $country) {
    echo $country->getName() . "<br />\n";
}

This works:

$cr = $this->get('continent_repository');

$continents = $cr->getContinentsWithCountries();
foreach ($continents[5]->getCountries()->getIterator() as $country) {
    echo $country->getName() . "<br />\n";
}
$licensedContinents = $cr->getAllowedContinentsWithCountries($this->getUser());
@doctrinebot

Comment created by @ocramius:

Quick note: it seems to me like you are filtering on fetch-joined associations. That's very dangerous.

[~beberlei] do you think we can put some kind of exception in an AST walker to avoid people from doing this? I think this will happen more and more often...

@doctrinebot

Comment created by flip101:

The problem happened because of using filtered associations in the previous query, and UnitofWork references the already existing objects. Ticket can be closed.

@doctrinebot

Comment created by @beberlei:

Marked as invalid

@doctrinebot

Issue was closed with resolution "Invalid"

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot closed this
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.