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

[DDC-2235] Fix for using a LEFT JOIN onto an entity with single table inheritance #656

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/Doctrine/ORM/Query/SqlWalker.php
Expand Up @@ -807,15 +807,18 @@ public function walkFromClause($fromClause)
* Walks down a RangeVariableDeclaration AST node, thereby generating the appropriate SQL.
*
* @param AST\RangeVariableDeclaration $rangeVariableDeclaration
* @param boolean $rootAlias
*
* @return string
*/
public function walkRangeVariableDeclaration($rangeVariableDeclaration)
public function walkRangeVariableDeclaration($rangeVariableDeclaration, $rootAlias = true)
{
$class = $this->em->getClassMetadata($rangeVariableDeclaration->abstractSchemaName);
$dqlAlias = $rangeVariableDeclaration->aliasIdentificationVariable;

$this->rootAliases[] = $dqlAlias;
if ($rootAlias) {
$this->rootAliases[] = $dqlAlias;
}

$sql = $class->getQuotedTableName($this->platform) . ' '
. $this->getSQLTableAlias($class->getTableName(), $dqlAlias);
Expand Down Expand Up @@ -1021,6 +1024,7 @@ public function walkJoin($join)
{
$joinType = $join->joinType;
$joinDeclaration = $join->joinAssociationDeclaration;
$joinAlias = $joinDeclaration->aliasIdentificationVariable;

$sql = ($joinType == AST\Join::JOIN_TYPE_LEFT || $joinType == AST\Join::JOIN_TYPE_LEFTOUTER)
? ' LEFT JOIN '
Expand All @@ -1033,8 +1037,15 @@ public function walkJoin($join)
? ' AND '
: ' ON ';

$sql .= $this->walkRangeVariableDeclaration($joinDeclaration)
$sql .= $this->walkRangeVariableDeclaration($joinDeclaration, false)
. $condExprConjunction . '(' . $this->walkConditionalExpression($join->conditionalExpression) . ')';

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

if ( ! empty($discrSql)) {
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline before this line

$sql .= ' AND (' . $discrSql . ')';
}

break;

case ($joinDeclaration instanceof \Doctrine\ORM\Query\AST\JoinAssociationDeclaration):
Expand Down
49 changes: 49 additions & 0 deletions tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
Expand Up @@ -1458,6 +1458,55 @@ public function testInheritanceTypeSingleTableInLeafClassWithEnabledForcePartial
);
}

/**
* @group DDC-2235
*/
public function testInheritanceTypeSingleTableIncludesDiscriminatorInJoinOnClause()
{
// Regression test for the bug
$this->assertSqlGeneration(
Copy link
Member

Choose a reason for hiding this comment

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

what happens outside the context of arbitrary joins?

such as select f from foo f join f.bar b with f.baz = ?

Copy link
Author

Choose a reason for hiding this comment

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

Good point, those shouldn't be affected at all - however i'll add some tests just to be sure!

Copy link
Member

Choose a reason for hiding this comment

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

Can you add them to the tests then please? :)

'SELECT c FROM Doctrine\Tests\Models\Company\CompanyEmployee e LEFT JOIN Doctrine\Tests\Models\Company\CompanyContract c WITH c.salesPerson = e.id',
"SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id LEFT JOIN company_contracts c0_ ON (c0_.salesPerson_id = c2_.id) AND (c0_.discr IN ('fix', 'flexible', 'flexultra'))"
);

// Ensure other WHERE predicates are passed through to the main WHERE clause
$this->assertSqlGeneration(
'SELECT c FROM Doctrine\Tests\Models\Company\CompanyEmployee e LEFT JOIN Doctrine\Tests\Models\Company\CompanyContract c WITH c.salesPerson = e.id WHERE e.salary > 1000',
"SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id LEFT JOIN company_contracts c0_ ON (c0_.salesPerson_id = c2_.id) AND (c0_.discr IN ('fix', 'flexible', 'flexultra')) WHERE c1_.salary > 1000"
);

// Test inner joins too
$this->assertSqlGeneration(
'SELECT c FROM Doctrine\Tests\Models\Company\CompanyEmployee e INNER JOIN Doctrine\Tests\Models\Company\CompanyContract c WITH c.salesPerson = e.id',
"SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id INNER JOIN company_contracts c0_ ON (c0_.salesPerson_id = c2_.id) AND (c0_.discr IN ('fix', 'flexible', 'flexultra'))"
);

// Test that the discriminator IN() predicate is still added into
// the where clause when not joining onto that table
$this->assertSqlGeneration(
'SELECT c FROM Doctrine\Tests\Models\Company\CompanyContract c LEFT JOIN Doctrine\Tests\Models\Company\CompanyEmployee e WITH e.id = c.salesPerson WHERE c.completed = true',
"SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_contracts c0_ LEFT JOIN company_employees c1_ INNER JOIN company_persons c2_ ON c1_.id = c2_.id ON (c2_.id = c0_.salesPerson_id) WHERE (c0_.completed = 1) AND c0_.discr IN ('fix', 'flexible', 'flexultra')"
);

// Test that the discriminator IN() predicate is still added
// into the where clause when not joining onto a single table inheritance entity
// via a join association
$this->assertSqlGeneration(
'SELECT c FROM Doctrine\Tests\Models\Company\CompanyContract c JOIN c.salesPerson s WHERE c.completed = true',
"SELECT c0_.id AS id0, c0_.completed AS completed1, c0_.fixPrice AS fixPrice2, c0_.hoursWorked AS hoursWorked3, c0_.pricePerHour AS pricePerHour4, c0_.maxPrice AS maxPrice5, c0_.discr AS discr6 FROM company_contracts c0_ INNER JOIN company_employees c1_ ON c0_.salesPerson_id = c1_.id LEFT JOIN company_persons c2_ ON c1_.id = c2_.id WHERE (c0_.completed = 1) AND c0_.discr IN ('fix', 'flexible', 'flexultra')"
);

// Test that when joining onto an entity using single table inheritance via
// a join association that the discriminator IN() predicate is placed
// into the ON clause of the join
$this->assertSqlGeneration(
'SELECT e, COUNT(c) FROM Doctrine\Tests\Models\Company\CompanyEmployee e JOIN e.contracts c WHERE e.department = :department',
"SELECT c0_.id AS id0, c0_.name AS name1, c1_.salary AS salary2, c1_.department AS department3, c1_.startDate AS startDate4, COUNT(c2_.id) AS sclr5, c0_.discr AS discr6 FROM company_employees c1_ INNER JOIN company_persons c0_ ON c1_.id = c0_.id INNER JOIN company_contract_employees c3_ ON c1_.id = c3_.employee_id INNER JOIN company_contracts c2_ ON c2_.id = c3_.contract_id AND c2_.discr IN ('fix', 'flexible', 'flexultra') WHERE c1_.department = ?",
array(),
array('department' => 'foobar')
);
}

/**
* @group DDC-1161
*/
Expand Down