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

fix(Hydrator): Array hydration having always primary key and selected relations #135

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e089921
fix: add failing test when aliasing a column of a joined relation
alquerci Apr 15, 2024
6a0fe33
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 15, 2024
0af3477
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 15, 2024
5572c06
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
2966018
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
3418f4f
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
6d65332
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
693ca6d
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
c61998f
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
8aef340
fixup! fix: add failing test when aliasing a column of a joined relation
alquerci Apr 16, 2024
3a5e0ff
Add identifier as minimum code to make test pass
alquerci Apr 16, 2024
aba7723
Add failing test for value of relation identifier
alquerci Apr 16, 2024
2cfcab3
restore previous code with issue column added twice with custom aliases
alquerci Apr 16, 2024
e2ab772
Add test to characterize previous behaviour and avoid regression
alquerci Apr 16, 2024
b65e2e6
fixup! Add test to characterize previous behaviour and avoid regression
alquerci Apr 16, 2024
7ba863c
fixup! Add test to characterize previous behaviour and avoid regression
alquerci Apr 16, 2024
3cf3894
fixup! Add test to characterize previous behaviour and avoid regression
alquerci Apr 16, 2024
7f0cf6c
fixup! Add test to characterize previous behaviour and avoid regression
alquerci Apr 16, 2024
606ef02
fixup! Add test to characterize previous behaviour and avoid regression
alquerci Apr 16, 2024
ca20e82
Revert "restore previous code with issue column added twice with cust…
alquerci Apr 16, 2024
bd0f138
Array hydration add identifier on select SQL of selected relation
alquerci Apr 17, 2024
15606f5
Array hydration add identifier on select SQL of selected relation
alquerci Apr 17, 2024
02314ab
Array hydration add identifier on select SQL of selected relation
alquerci Apr 17, 2024
fc58224
restore previous code with issue column added twice with custom aliases
alquerci Apr 17, 2024
b0df112
Revert "restore previous code with issue column added twice with cust…
alquerci Apr 17, 2024
5820e8d
add select relation identifier for on demand hydration
alquerci Apr 17, 2024
f48f90e
fixup! add select relation identifier for on demand hydration
alquerci Apr 17, 2024
d47deb6
fixup! add select relation identifier for on demand hydration
alquerci Apr 17, 2024
153b81c
fixup! add select relation identifier for on demand hydration
alquerci Apr 17, 2024
6950970
fixup! add select relation identifier for on demand hydration
alquerci Apr 17, 2024
03dd3bf
fixup! add select relation identifier for on demand hydration
alquerci Apr 17, 2024
3291eaa
restore previous code with issue column added twice with custom aliases
alquerci Apr 17, 2024
a33cdbc
Revert "restore previous code with issue column added twice with cust…
alquerci Apr 17, 2024
5156720
only add relation identifier when at least one relation column is sel…
alquerci Apr 17, 2024
1c56024
fixup! only add relation identifier when at least one relation column…
alquerci Apr 17, 2024
3777119
add select relation identifier for array and record hierarchy
alquerci Apr 17, 2024
d7cb5dd
fixup! add select relation identifier for array and record hierarchy
alquerci Apr 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion lib/Doctrine/Hydrator/ArrayDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,37 @@ public function setLastElement(&$prev, &$coll, $index, $dqlAlias, $oneToOne)
}
}
}
}

protected function beforeAddingAggregateValue($rowData, $cache, $dqlAlias, $value)
{
$rowData = $this->addSelectedRelationToRowData($rowData, $dqlAlias, $cache);

$rowData = $this->addIdentifierColumnToRowData($rowData, $cache, $dqlAlias, $value);

return $rowData;
}

private function addSelectedRelationToRowData($rowData, $dqlAlias, $cache)
{
if (!isset($rowData[$dqlAlias])) {
if ($cache['isRelation']) {
$rowData[$dqlAlias] = array();

foreach ($cache['identifiers'] as $identifierField) {
$rowData[$dqlAlias][$identifierField] = null;
}
}
}

return $rowData;
}

private function addIdentifierColumnToRowData($rowData, $cache, $dqlAlias, $value)
{
if ($cache['isIdentifier'] && !isset($rowData[$dqlAlias][$cache['columnName']])) {
$rowData[$dqlAlias][$cache['columnName']] = $value;
}

return $rowData;
}
}
34 changes: 26 additions & 8 deletions lib/Doctrine/Hydrator/Graph.php
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,25 @@ protected function _gatherRowData(&$data, &$cache, &$id, &$nonemptyComponents)
$fieldName = $table->getFieldName($last);
$cache[$key]['fieldName'] = $fieldName;

$cache[$key]['identifiers'] = (array) $table->getIdentifier();

$cache[$key]['isRelation'] = isset($this->_queryComponents[$cache[$key]['dqlAlias']]['relation']);

if (isset($this->_queryComponents[$cache[$key]['dqlAlias']]['agg'][$fieldName])) {
$cache[$key]['isAgg'] = true;
$cache[$key]['aliasName'] = $this->_queryComponents[$cache[$key]['dqlAlias']]['agg'][$fieldName];
} else {
$cache[$key]['isAgg'] = false;
$cache[$key]['aliasName'] = $fieldName;
}

if (isset($this->_queryComponents[$cache[$key]['dqlAlias']]['agg_field'][$last])) {
$fieldName = $this->_queryComponents[$cache[$key]['dqlAlias']]['agg_field'][$last];
$cache[$key]['columnName'] = $this->_queryComponents[$cache[$key]['dqlAlias']]['agg_field'][$last];
} else {
$cache[$key]['columnName'] = $fieldName;
}

if ($table->isIdentifier($fieldName)) {
if ($table->isIdentifier($cache[$key]['columnName'])) {
$cache[$key]['isIdentifier'] = true;
} else {
$cache[$key]['isIdentifier'] = false;
Expand All @@ -334,12 +348,8 @@ protected function _gatherRowData(&$data, &$cache, &$id, &$nonemptyComponents)
$map = $this->_queryComponents[$cache[$key]['dqlAlias']];
$table = $map['table'];
$dqlAlias = $cache[$key]['dqlAlias'];
$fieldName = $cache[$key]['fieldName'];
$agg = false;
if (isset($this->_queryComponents[$dqlAlias]['agg'][$fieldName])) {
$fieldName = $this->_queryComponents[$dqlAlias]['agg'][$fieldName];
$agg = true;
}
$fieldName = $cache[$key]['aliasName'];
$agg = $cache[$key]['isAgg'];

if ($cache[$key]['isIdentifier']) {
$id[$dqlAlias] .= '|' . $value;
Expand All @@ -355,7 +365,10 @@ protected function _gatherRowData(&$data, &$cache, &$id, &$nonemptyComponents)
// Hydrate aggregates in to the root component as well.
// So we know that all aggregate values will always be available in the root component
if ($agg) {
$rowData = $this->beforeAddingAggregateValue($rowData, $cache[$key], $dqlAlias, $preparedValue);

$rowData[$this->_rootAlias][$fieldName] = $preparedValue;

if (isset($rowData[$dqlAlias])) {
$rowData[$dqlAlias][$fieldName] = $preparedValue;
}
Expand All @@ -371,6 +384,11 @@ protected function _gatherRowData(&$data, &$cache, &$id, &$nonemptyComponents)
return $rowData;
}

protected function beforeAddingAggregateValue($rowData, $cache, $dqlAlias, $value)
{
return $rowData;
}

abstract public function getElementCollection($component);

abstract public function registerCollection($coll);
Expand Down
42 changes: 37 additions & 5 deletions lib/Doctrine/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,7 @@ public function processPendingFields($componentAlias)
if (in_array('*', $fields)) {
$fields = $table->getFieldNames();
} else {
$driverClassName = $this->_hydrator->getHydratorDriverClassName();
// only auto-add the primary key fields if this query object is not
// a subquery of another query object or we're using a child of the Object Graph
// hydrator
if ( ! $this->_isSubquery && is_subclass_of($driverClassName, 'Doctrine_Hydrator_Graph')) {
if ($this->shouldAutoSelectIdentifiers()) {
$fields = array_unique(array_merge((array) $table->getIdentifier(), $fields));
}
}
Expand Down Expand Up @@ -504,6 +500,18 @@ public function processPendingFields($componentAlias)
return implode(', ', $sql);
}

/*
* only auto-add the primary key fields if this query object is not
* a subquery of another query object or we're using
* a child of the Object Graph hydrator
*/
private function shouldAutoSelectIdentifiers()
{
$driverClassName = $this->_hydrator->getHydratorDriverClassName();

return ! $this->_isSubquery && is_subclass_of($driverClassName, 'Doctrine_Hydrator_Graph');
}

/**
* Parses a nested field
* <code>
Expand Down Expand Up @@ -615,6 +623,7 @@ public function parseSelect($dql)

$terms = $this->_tokenizer->sqlExplode($reference, ' ');
$pos = strpos($terms[0], '(');
$isColumnSelect = $pos === false;

if (count($terms) > 1 || $pos !== false) {
$expression = array_shift($terms);
Expand Down Expand Up @@ -647,6 +656,8 @@ public function parseSelect($dql)
$this->_expressionMap[$alias][0] = $expression;

$this->_queryComponents[$componentAlias]['agg'][$index] = $alias;
$this->_queryComponents[$componentAlias]['has_selected_column'] ??= false;
$this->_queryComponents[$componentAlias]['has_selected_column'] |= $isColumnSelect;

if (preg_match('/^([^\(]+)\.(\'?)(.*?)(\'?)$/', $expression, $field)) {
$this->_queryComponents[$componentAlias]['agg_field'][$index] = $field[3];
Expand All @@ -668,6 +679,27 @@ public function parseSelect($dql)
$this->_pendingFields[$componentAlias][] = $field;
}
}

$this->appendRelationIdentifierOnSqlSelect();
}

private function appendRelationIdentifierOnSqlSelect()
{
if ($this->shouldAutoSelectIdentifiers()) {
foreach ($this->_queryComponents as $componentAlias => $queryComponent) {
if (
isset($queryComponent['relation'])
&& isset($queryComponent['agg'])
&& !empty($queryComponent['has_selected_column'])
) {
$table = $queryComponent['table'];

foreach ((array) $table->getIdentifier() as $field) {
$this->_pendingFields[$componentAlias][] = $field;
}
}
}
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Ticket/585TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function test_hydrateArrayShallow_withAllColumnsAliased_thenResultsHasAll
public function test_hydrateArray_withAllColumnsAliased_thenResultsHasAllRecords()
{
$hydrateType = Doctrine_Core::HYDRATE_ARRAY;
$expectedKeys = array('aliasId', 'aliasName');
$expectedKeys = array('id', 'aliasId', 'aliasName');

$this->doTestWithAllColumnsAliased($hydrateType, $expectedKeys);
}
Expand Down
177 changes: 177 additions & 0 deletions tests/Ticket/GH134TestCase.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
<?php

class Doctrine_Ticket_GH134_TestCase extends Doctrine_UnitTestCase
{
public function test_addIdentifierForSelectedRelation_withAliases()
{
foreach ($this->provideIdentifierAndRelationWithAliasData() as [$hydrateType, $expectedSql, $expectedFirstResult]) {
$query = Doctrine_Query::create()
->select('u.id, e.address as aliasAddress')
->from('User u')
->innerJoin('u.Email e')
;

$results = $query->execute(array(), $hydrateType);

$this->assertEqual($expectedSql, $query->getSqlQuery());

foreach ($results as $firstResult) {
break;
}

if (Doctrine_Core::HYDRATE_RECORD === $hydrateType
|| Doctrine_Core::HYDRATE_ON_DEMAND === $hydrateType
) {
$firstResult = $firstResult->toArray();

$firstResult = array_intersect_key($firstResult, $expectedFirstResult);
ksort($firstResult);
ksort($expectedFirstResult);

if (isset($firstResult['Email'])) {
$firstResult['Email'] = array_intersect_key($firstResult['Email'], $expectedFirstResult['Email']);
ksort($firstResult['Email']);
ksort($expectedFirstResult['Email']);
}
}

$this->assertEqual($expectedFirstResult, $firstResult);

if (Doctrine_Core::HYDRATE_ON_DEMAND !== $hydrateType) {
$this->assertEqual(count($this->users), count($results));
}
}
}

private function provideIdentifierAndRelationWithAliasData()
{
yield [
Doctrine_Core::HYDRATE_ARRAY,
'SELECT e.id AS e__id, e2.id AS e2__id, e2.address AS e2__0 FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'id' => '4',
'aliasAddress' => '[email protected]',
'Email' => array (
'id' => 1,
'aliasAddress' => '[email protected]',
),
),
];

yield [
Doctrine_Core::HYDRATE_ARRAY_SHALLOW,
'SELECT e.id AS e__id, e2.address AS e2__0 FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'id' => '4',
'aliasAddress' => '[email protected]',
),
];

yield [
Doctrine_Core::HYDRATE_SCALAR,
'SELECT e.id AS e__id, e2.address AS e2__0 FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'u_id' => '4',
'e_aliasAddress' => '[email protected]',
),
];

yield [
Doctrine_Core::HYDRATE_RECORD,
'SELECT e.id AS e__id, e2.id AS e2__id, e2.address AS e2__0 FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'id' => '4',
'aliasAddress' => '[email protected]',
'Email' => array (
'id' => 1,
'aliasAddress' => '[email protected]',
),
),
];

yield [
Doctrine_Core::HYDRATE_ON_DEMAND,
'SELECT e.id AS e__id, e2.id AS e2__id, e2.address AS e2__0 FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'id' => '4',
'aliasAddress' => '[email protected]',
'Email' => array (
'id' => 1,
'aliasAddress' => '[email protected]',
),
),
];
}

public function test_addIdentifierForSelectedRelation_withoutAlias()
{
foreach ($this->provideIdentifierAndRelationWithoutAliasData() as [$hydrateType, $expectedSql, $expectedFirstResult]) {
$query = Doctrine_Query::create()
->select('u.id, e.address')
->from('User u')
->innerJoin('u.Email e')
;

$results = $query->execute(array(), $hydrateType);

$this->assertEqual($expectedSql, $query->getSqlQuery());

$this->assertEqual($expectedFirstResult, $results[0]);
$this->assertEqual(count($this->users), count($results));
}
}

private function provideIdentifierAndRelationWithoutAliasData()
{
yield [
Doctrine_Core::HYDRATE_ARRAY,
'SELECT e.id AS e__id, e2.id AS e2__id, e2.address AS e2__address FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'id' => '4',
'Email' => array (
'id' => 1,
'address' => '[email protected]',
),
),
];

yield [
Doctrine_Core::HYDRATE_ARRAY_SHALLOW,
'SELECT e.id AS e__id, e2.address AS e2__address FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'id' => '4',
'address' => '[email protected]',
),
];

yield [
Doctrine_Core::HYDRATE_SCALAR,
'SELECT e.id AS e__id, e2.address AS e2__address FROM entity e INNER JOIN email e2 ON e.email_id = e2.id WHERE (e.type = 0)',
array (
'u_id' => '4',
'e_address' => '[email protected]',
),
];
}

public function test_columnAliasWithSameNameAsIdentifier_willKeepOverwriteIdentifierByAlias()
{
foreach ($this->provideKeepOverwriteIdentifierWithAliasData() as [$hydrateType, $checkArrayIndex]) {
$query = Doctrine_Query::create()
->select('99 as id, u.id as aliasId')
->from('User u')
;

$results = $query->execute(array(), $hydrateType);

$this->assertEqual(99, $results[0][$checkArrayIndex]);
}
}

private function provideKeepOverwriteIdentifierWithAliasData()
{
yield [Doctrine_Core::HYDRATE_SCALAR, 'u_id'];
yield [Doctrine_Core::HYDRATE_ARRAY, 'id'];
yield [Doctrine_Core::HYDRATE_ARRAY_SHALLOW, 'id'];
}
}
Loading