Skip to content

Commit

Permalink
Got rid of redundant JOINs/WHEREs of WhereInWalker
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergii Dolgushev authored and Sergii Dolgushev committed Aug 23, 2023
1 parent 5577d51 commit e60ba0f
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 58 deletions.
62 changes: 28 additions & 34 deletions lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use Doctrine\ORM\Query\AST\ArithmeticExpression;
use Doctrine\ORM\Query\AST\ConditionalExpression;
use Doctrine\ORM\Query\AST\ConditionalFactor;
use Doctrine\ORM\Query\AST\ConditionalPrimary;
use Doctrine\ORM\Query\AST\ConditionalTerm;
use Doctrine\ORM\Query\AST\InListExpression;
Expand All @@ -18,7 +17,6 @@
use Doctrine\ORM\Query\AST\WhereClause;
use Doctrine\ORM\Query\TreeWalkerAdapter;
use RuntimeException;

use function count;
use function reset;

Expand Down Expand Up @@ -82,39 +80,35 @@ public function walkSelectStatement(SelectStatement $AST)

$conditionalPrimary = new ConditionalPrimary();
$conditionalPrimary->simpleConditionalExpression = $expression;
if ($AST->whereClause) {
if ($AST->whereClause->conditionalExpression instanceof ConditionalTerm) {
$AST->whereClause->conditionalExpression->conditionalFactors[] = $conditionalPrimary;
} elseif ($AST->whereClause->conditionalExpression instanceof ConditionalPrimary) {
$AST->whereClause->conditionalExpression = new ConditionalExpression(
[
new ConditionalTerm(
[
$AST->whereClause->conditionalExpression,
$conditionalPrimary,
]
),
]
);
} elseif (
$AST->whereClause->conditionalExpression instanceof ConditionalExpression
|| $AST->whereClause->conditionalExpression instanceof ConditionalFactor
) {
$tmpPrimary = new ConditionalPrimary();
$tmpPrimary->conditionalExpression = $AST->whereClause->conditionalExpression;
$AST->whereClause->conditionalExpression = new ConditionalTerm(
[
$tmpPrimary,
$conditionalPrimary,
]
);
$AST->whereClause = new WhereClause(
new ConditionalExpression(
[new ConditionalTerm([$conditionalPrimary])]
)
);

if ($this->onlyFromIsUsedInSelect($AST)) {
foreach ($AST->fromClause->identificationVariableDeclarations as $f) {
$f->joins = [];
}
} else {
$AST->whereClause = new WhereClause(
new ConditionalExpression(
[new ConditionalTerm([$conditionalPrimary])]
)
);
}
}

/**
* @return bool
*/
private function onlyFromIsUsedInSelect(SelectStatement $AST)
{
$fromAliases = [];
foreach ($AST->fromClause->identificationVariableDeclarations as $f) {
$fromAliases[] = $f->rangeVariableDeclaration->aliasIdentificationVariable;
}

foreach ($AST->selectClause->selectExpressions as $s) {
if (!in_array($s->expression, $fromAliases)) {
return false;
}
}

return true;
}
}
15 changes: 0 additions & 15 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2642,21 +2642,6 @@
<code>$orderByClause</code>
</PropertyNotSetInConstructor>
</file>
<file src="lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php">
<DocblockTypeContradiction>
<code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalExpression
|| $AST->whereClause->conditionalExpression instanceof ConditionalFactor]]></code>
<code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalFactor]]></code>
<code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalPrimary]]></code>
</DocblockTypeContradiction>
<PossiblyInvalidPropertyAssignmentValue>
<code><![CDATA[$AST->whereClause->conditionalExpression]]></code>
</PossiblyInvalidPropertyAssignmentValue>
<RedundantConditionGivenDocblockType>
<code><![CDATA[$AST->whereClause->conditionalExpression instanceof ConditionalExpression
|| $AST->whereClause->conditionalExpression instanceof ConditionalFactor]]></code>
</RedundantConditionGivenDocblockType>
</file>
<file src="lib/Doctrine/ORM/Tools/SchemaTool.php">
<ArgumentTypeCoercion>
<code>$classes</code>
Expand Down
28 changes: 19 additions & 9 deletions tests/Doctrine/Tests/ORM/Tools/Pagination/WhereInWalkerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,52 @@ public static function exampleQueries(): Generator

yield 'single WHERE condition' => [
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g WHERE 1 = 1',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE 1 = 1 AND u0_.id IN (?)',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'multiple WHERE conditions with AND' => [
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g WHERE 1 = 1 AND 2 = 2',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE 1 = 1 AND 2 = 2 AND u0_.id IN (?)',
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'multiple WHERE conditions with OR' => [
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g WHERE 1 = 1 OR 2 = 2',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE (1 = 1 OR 2 = 2) AND u0_.id IN (?)',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'multiple WHERE conditions with mixed clauses 1' => [
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g WHERE (1 = 1 OR 2 = 2) AND 3 = 3',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE (1 = 1 OR 2 = 2) AND 3 = 3 AND u0_.id IN (?)',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'multiple WHERE conditions with mixed clauses 2' => [
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g WHERE 1 = 1 AND 2 = 2 OR 3 = 3',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE (1 = 1 AND 2 = 2 OR 3 = 3) AND u0_.id IN (?)',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'WHERE NOT condition' => [
'SELECT u, g FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g WHERE NOT 1 = 2',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE (NOT 1 = 2) AND u0_.id IN (?)',
'SELECT u0_.id AS id_0, g1_.id AS id_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'join used in select' => [
'SELECT u, sum(g.id) FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g',
'SELECT u0_.id AS id_0, sum(g1_.id) AS sclr_1 FROM User u0_ INNER JOIN user_group u2_ ON u0_.id = u2_.user_id INNER JOIN groups g1_ ON g1_.id = u2_.group_id WHERE u0_.id IN (?)',
];

yield 'join not used in select' => [
'SELECT u FROM Doctrine\Tests\ORM\Tools\Pagination\User u JOIN u.groups g',
'SELECT u0_.id AS id_0 FROM User u0_ WHERE u0_.id IN (?)',
];

yield 'arbitary join with no WHERE' => [
'SELECT p FROM Doctrine\Tests\ORM\Tools\Pagination\BlogPost p JOIN Doctrine\Tests\ORM\Tools\Pagination\Category c WITH p.category = c',
'SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ INNER JOIN Category c1_ ON (b0_.category_id = c1_.id) WHERE b0_.id IN (?)',
'SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ WHERE b0_.id IN (?)',
];

yield 'arbitary join with single WHERE' => [
'SELECT p FROM Doctrine\Tests\ORM\Tools\Pagination\BlogPost p JOIN Doctrine\Tests\ORM\Tools\Pagination\Category c WITH p.category = c WHERE 1 = 1',
'SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ INNER JOIN Category c1_ ON (b0_.category_id = c1_.id) WHERE 1 = 1 AND b0_.id IN (?)',
'SELECT b0_.id AS id_0, b0_.author_id AS author_id_1, b0_.category_id AS category_id_2 FROM BlogPost b0_ WHERE b0_.id IN (?)',
];
}
}

0 comments on commit e60ba0f

Please sign in to comment.