Skip to content

Commit

Permalink
[2.0][DDC-614] Added support to multiple FROM identification variable…
Browse files Browse the repository at this point in the history 
…s. Also, fixed bug with missing lock on subselect.
  • Loading branch information
Guilherme Blanco committed Jul 20, 2010
1 parent a050030 commit 2c28872
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 51 deletions.
116 changes: 67 additions & 49 deletions lib/Doctrine/ORM/Query/SqlWalker.php
View file
Expand Up @@ -77,9 +77,8 @@ class SqlWalker implements TreeWalker

/**
* The DQL alias of the root class of the currently traversed query.
* TODO: May need to be turned into a stack for usage in subqueries
*/
private $_currentRootAlias;
private $_rootAliases = array();

/**
* Flag that indicates whether to generate SQL table aliases in the SQL.
Expand Down Expand Up @@ -187,6 +186,7 @@ public function getExecutor($AST)
/**
* Generates a unique, short SQL table alias.
*
* @param string $tableName Table name
* @param string $dqlAlias The DQL alias.
* @return string Generated table alias.
*/
Expand Down Expand Up @@ -308,14 +308,15 @@ private function _generateOrderedCollectionOrderByItems()
/**
* Generates a discriminator column SQL condition for the class with the given DQL alias.
*
* @param string $dqlAlias
* @param array $dqlAliases List of root DQL aliases to inspect for discriminator restrictions.
* @return string
*/
private function _generateDiscriminatorColumnConditionSQL($dqlAlias)
private function _generateDiscriminatorColumnConditionSQL(array $dqlAliases)
{
$encapsulate = false;
$sql = '';

if ($dqlAlias) {
foreach ($dqlAliases as $dqlAlias) {
$class = $this->_queryComponents[$dqlAlias]['metadata'];

if ($class->isInheritanceTypeSingleTable()) {
Expand All @@ -329,14 +330,18 @@ private function _generateDiscriminatorColumnConditionSQL($dqlAlias)
$values[] = $conn->quote($this->_em->getClassMetadata($subclassName)->discriminatorValue);
}

$sql .= (($this->_useSqlTableAliases)
? $this->getSqlTableAlias($class->table['name'], $dqlAlias) . '.' : ''
) . $class->discriminatorColumn['name']
. ' IN (' . implode(', ', $values) . ')';
if ($sql != '') {
$sql .= ' AND ';
$encapsulate = true;
}

$sql .= ($sql != '' ? ' AND ' : '')
. (($this->_useSqlTableAliases) ? $this->getSqlTableAlias($class->table['name'], $dqlAlias) . '.' : '')
. $class->discriminatorColumn['name'] . ' IN (' . implode(', ', $values) . ')';
}
}

return $sql;
return ($encapsulate) ? '(' . $sql . ')' : $sql;
}

/**
Expand All @@ -351,7 +356,7 @@ public function walkSelectStatement(AST\SelectStatement $AST)

if (($whereClause = $AST->whereClause) !== null) {
$sql .= $this->walkWhereClause($whereClause);
} else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_currentRootAlias)) !== '') {
} else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_rootAliases)) !== '') {
$sql .= ' WHERE ' . $discSql;
}

Expand Down Expand Up @@ -399,7 +404,7 @@ public function walkUpdateStatement(AST\UpdateStatement $AST)

if (($whereClause = $AST->whereClause) !== null) {
$sql .= $this->walkWhereClause($whereClause);
} else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_currentRootAlias)) !== '') {
} else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_rootAliases)) !== '') {
$sql .= ' WHERE ' . $discSql;
}

Expand All @@ -419,7 +424,7 @@ public function walkDeleteStatement(AST\DeleteStatement $AST)

if (($whereClause = $AST->whereClause) !== null) {
$sql .= $this->walkWhereClause($whereClause);
} else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_currentRootAlias)) !== '') {
} else if (($discSql = $this->_generateDiscriminatorColumnConditionSQL($this->_rootAliases)) !== '') {
$sql .= ' WHERE ' . $discSql;
}

Expand Down Expand Up @@ -599,34 +604,40 @@ public function walkSelectClause($selectClause)
*/
public function walkFromClause($fromClause)
{
$sql = ' FROM ';
$identificationVarDecls = $fromClause->identificationVariableDeclarations;
$firstIdentificationVarDecl = $identificationVarDecls[0];
$rangeDecl = $firstIdentificationVarDecl->rangeVariableDeclaration;
$dqlAlias = $rangeDecl->aliasIdentificationVariable;
$sqlParts = array();

$this->_currentRootAlias = $dqlAlias;
foreach ($identificationVarDecls as $identificationVariableDecl) {
$sql = '';

$class = $this->_em->getClassMetadata($rangeDecl->abstractSchemaName);
$sql .= $class->getQuotedTableName($this->_platform) . ' '
. $this->getSqlTableAlias($class->table['name'], $dqlAlias);
$rangeDecl = $identificationVariableDecl->rangeVariableDeclaration;
$dqlAlias = $rangeDecl->aliasIdentificationVariable;

$this->_rootAliases[] = $dqlAlias;

if ($class->isInheritanceTypeJoined()) {
$sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias);
}
$class = $this->_em->getClassMetadata($rangeDecl->abstractSchemaName);
$sql .= $class->getQuotedTableName($this->_platform) . ' '
. $this->getSqlTableAlias($class->table['name'], $dqlAlias);

foreach ($firstIdentificationVarDecl->joinVariableDeclarations as $joinVarDecl) {
$sql .= $this->walkJoinVariableDeclaration($joinVarDecl);
}
if ($class->isInheritanceTypeJoined()) {
$sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias);
}

if ($firstIdentificationVarDecl->indexBy) {
$this->_rsm->addIndexBy(
$firstIdentificationVarDecl->indexBy->simpleStateFieldPathExpression->identificationVariable,
$firstIdentificationVarDecl->indexBy->simpleStateFieldPathExpression->parts[0]
);
foreach ($identificationVariableDecl->joinVariableDeclarations as $joinVarDecl) {
$sql .= $this->walkJoinVariableDeclaration($joinVarDecl);
}

if ($identificationVariableDecl->indexBy) {
$this->_rsm->addIndexBy(
$identificationVariableDecl->indexBy->simpleStateFieldPathExpression->identificationVariable,
$identificationVariableDecl->indexBy->simpleStateFieldPathExpression->parts[0]
);
}

$sqlParts[] = $this->_platform->appendLockHint($sql, $this->_query->getHint(Query::HINT_LOCK_MODE));
}

return $this->_platform->appendLockHint($sql, $this->_query->getHint(Query::HINT_LOCK_MODE));
return ' FROM ' . implode(', ', $sqlParts);
}

/**
Expand Down Expand Up @@ -802,7 +813,7 @@ public function walkJoinVariableDeclaration($joinVarDecl)
$sql .= ' AND (' . $this->walkConditionalExpression($condExpr) . ')';
}

$discrSql = $this->_generateDiscriminatorColumnConditionSQL($joinedDqlAlias);
$discrSql = $this->_generateDiscriminatorColumnConditionSQL(array($joinedDqlAlias));

if ($discrSql) {
$sql .= ' AND ' . $discrSql;
Expand Down Expand Up @@ -1045,23 +1056,30 @@ public function walkSubselect($subselect)
public function walkSubselectFromClause($subselectFromClause)
{
$identificationVarDecls = $subselectFromClause->identificationVariableDeclarations;
$firstIdentificationVarDecl = $identificationVarDecls[0];
$rangeDecl = $firstIdentificationVarDecl->rangeVariableDeclaration;
$dqlAlias = $rangeDecl->aliasIdentificationVariable;
$sqlParts = array ();

$class = $this->_em->getClassMetadata($rangeDecl->abstractSchemaName);
$sql = ' FROM ' . $class->getQuotedTableName($this->_platform) . ' '
. $this->getSqlTableAlias($class->table['name'], $dqlAlias);
foreach ($identificationVarDecls as $subselectIdVarDecl) {
$sql = '';

if ($class->isInheritanceTypeJoined()) {
$sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias);
}
$rangeDecl = $subselectIdVarDecl->rangeVariableDeclaration;
$dqlAlias = $rangeDecl->aliasIdentificationVariable;

$class = $this->_em->getClassMetadata($rangeDecl->abstractSchemaName);
$sql .= $class->getQuotedTableName($this->_platform) . ' '
. $this->getSqlTableAlias($class->table['name'], $dqlAlias);

foreach ($firstIdentificationVarDecl->joinVariableDeclarations as $joinVarDecl) {
$sql .= $this->walkJoinVariableDeclaration($joinVarDecl);
if ($class->isInheritanceTypeJoined()) {
$sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias);
}

foreach ($subselectIdVarDecl->joinVariableDeclarations as $joinVarDecl) {
$sql .= $this->walkJoinVariableDeclaration($joinVarDecl);
}

$sqlParts[] = $this->_platform->appendLockHint($sql, $this->_query->getHint(Query::HINT_LOCK_MODE));
}

return $sql;
return ' FROM ' . implode(', ', $sqlParts);
}

/**
Expand Down Expand Up @@ -1161,7 +1179,7 @@ public function walkDeleteClause(AST\DeleteClause $deleteClause)
$sql .= ' ' . $this->getSqlTableAlias($class->getTableName());
}

$this->_currentRootAlias = $deleteClause->aliasIdentificationVariable;
$this->_rootAliases[] = $deleteClause->aliasIdentificationVariable;

return $sql;
}
Expand All @@ -1182,7 +1200,7 @@ public function walkUpdateClause($updateClause)
$sql .= ' ' . $this->getSqlTableAlias($class->getTableName());
}

$this->_currentRootAlias = $updateClause->aliasIdentificationVariable;
$this->_rootAliases[] = $updateClause->aliasIdentificationVariable;

$sql .= ' SET ' . implode(
', ', array_map(array($this, 'walkUpdateItem'), $updateClause->updateItems)
Expand Down Expand Up @@ -1227,7 +1245,7 @@ public function walkUpdateItem($updateItem)
*/
public function walkWhereClause($whereClause)
{
$discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_currentRootAlias);
$discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_rootAliases);
$condSql = $this->walkConditionalExpression($whereClause->conditionalExpression);

return ' WHERE ' . (( ! $discrSql) ? $condSql : '(' . $condSql . ') AND ' . $discrSql);
Expand Down
5 changes: 5 additions & 0 deletions tests/Doctrine/Tests/ORM/Query/LanguageRecognitionTest.php
View file
Expand Up @@ -79,6 +79,11 @@ public function testSelectSingleComponentWithMultipleColumns()
$this->assertValidDql('SELECT u.name, u.username FROM Doctrine\Tests\Models\CMS\CmsUser u');
}

public function testSelectMultipleComponentsUsingMultipleFrom()
{
$this->assertValidDql('SELECT u, p FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsPhonenumber p WHERE u = p.user');
}

public function testSelectMultipleComponentsWithAsterisk()
{
$this->assertValidDql('SELECT u, p FROM Doctrine\Tests\Models\CMS\CmsUser u JOIN u.phonenumbers p');
Expand Down
20 changes: 18 additions & 2 deletions tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
View file
Expand Up @@ -72,6 +72,14 @@ public function testSupportsSelectForMultipleColumnsOfASingleComponent()
);
}

public function testSupportsSelectUsingMultipleFromComponents()
{
$this->assertSqlGeneration(
'SELECT u, p FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsPhonenumber p WHERE u = p.user',
'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c1_.phonenumber AS phonenumber4 FROM cms_users c0_, cms_phonenumbers c1_ WHERE c0_.id = c1_.user_id'
);
}

public function testSupportsSelectWithCollectionAssociationJoin()
{
$this->assertSqlGeneration(
Expand Down Expand Up @@ -187,8 +195,16 @@ public function testSupportsArithmeticExpressionsInWherePart()
public function testSupportsMultipleEntitiesInFromClause()
{
$this->assertSqlGeneration(
'SELECT u, a FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsArticle a WHERE u.id = a.user.id',
'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c1_.id AS id4, c1_.topic AS topic5, c1_.text AS text6, c1_.version AS version7 FROM cms_users c0_ INNER JOIN cms_users c2_ ON c1_.user_id = c2_.id WHERE c0_.id = c2_.id'
'SELECT u, a FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsArticle a JOIN a.user u2 WHERE u.id = u2.id',
'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c1_.id AS id4, c1_.topic AS topic5, c1_.text AS text6, c1_.version AS version7 FROM cms_users c0_, cms_articles c1_ INNER JOIN cms_users c2_ ON c1_.user_id = c2_.id WHERE c0_.id = c2_.id'
);
}

public function testSupportsMultipleEntitiesInFromClauseUsingPathExpression()
{
$this->assertSqlGeneration(
'SELECT u, a FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsArticle a WHERE u.id = a.user',
'SELECT c0_.id AS id0, c0_.status AS status1, c0_.username AS username2, c0_.name AS name3, c1_.id AS id4, c1_.topic AS topic5, c1_.text AS text6, c1_.version AS version7 FROM cms_users c0_, cms_articles c1_ WHERE c0_.id = c1_.user_id'
);
}

Expand Down

3 comments on commit 2c28872

@beberlei
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing additional hydration aswell as functional tests. Does it work that easily?

For example, say i do ->iterate(), does it return the Many Models in the FROM clause at $row[0] and $row[1] ?

@guilhermeblanco
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beberlei Yes, everything related to Hydration is already implemented and already contains functional tests, as I already spoke to @romanb previously.

@beberlei
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah cool, great!

Please sign in to comment.