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.

Sign up for GitHub

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

NEW Add abstraction for sql RIGHT JOIN #10954

Merged
merged 1 commit into from
Sep 24, 2023
Merged

NEW Add abstraction for sql RIGHT JOIN #10954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions src/ORM/DataQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,26 @@ public function leftJoin($table, $onClause, $alias = null, $order = 20, $paramet
return $this;
}

/**
* Add a RIGHT JOIN clause to this query.
*
* @param string $table The unquoted table to join to.
* @param string $onClause The filter for the join (escaped SQL statement).
* @param string $alias An optional alias name (unquoted)
* @param int $order A numerical index to control the order that joins are added to the query; lower order values
* will cause the query to appear first. The default is 20, and joins created automatically by the
* ORM have a value of 10.
* @param array $parameters Any additional parameters if the join is a parameterised subquery
* @return $this
*/
public function rightJoin($table, $onClause, $alias = null, $order = 20, $parameters = [])
{
if ($table) {
$this->query->addRightJoin($table, $onClause, $alias, $order, $parameters);
}
return $this;
}

/**
* Prefix of all joined table aliases. E.g. ->filter('Banner.Image.Title)'
* Will join the Banner, and then Image relations
Expand Down
40 changes: 28 additions & 12 deletions src/ORM/Queries/SQLConditionalExpression.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,25 @@ public function useConjunction()
*/
public function addLeftJoin($table, $onPredicate, $tableAlias = '', $order = 20, $parameters = [])
{
if (!$tableAlias) {
$tableAlias = $table;
}
$this->from[$tableAlias] = [
'type' => 'LEFT',
'table' => $table,
'filter' => [$onPredicate],
'order' => $order,
'parameters' => $parameters
];
return $this;
return $this->addJoin($table, 'LEFT', $onPredicate, $tableAlias, $order, $parameters);
}

/**
* Add a RIGHT JOIN criteria to the tables list.
*
* @param string $table Unquoted table name
* @param string $onPredicate The "ON" SQL fragment in a "RIGHT JOIN ... AS ... ON ..." statement, Needs to be valid
* (quoted) SQL.
* @param string $tableAlias Optional alias which makes it easier to identify and replace joins later on
* @param int $order A numerical index to control the order that joins are added to the query; lower order values
* will cause the query to appear first. The default is 20, and joins created automatically by the
* ORM have a value of 10.
* @param array $parameters Any additional parameters if the join is a parameterized subquery
* @return $this Self reference
*/
public function addRightJoin($table, $onPredicate, $tableAlias = '', $order = 20, $parameters = [])
{
return $this->addJoin($table, 'RIGHT', $onPredicate, $tableAlias, $order, $parameters);
}

/**
Expand All @@ -163,12 +171,20 @@ public function addLeftJoin($table, $onPredicate, $tableAlias = '', $order = 20,
* @return $this Self reference
*/
public function addInnerJoin($table, $onPredicate, $tableAlias = null, $order = 20, $parameters = [])
{
return $this->addJoin($table, 'INNER', $onPredicate, $tableAlias, $order, $parameters);
}

/**
* Add a JOIN criteria
*/
private function addJoin($table, $type, $onPredicate, $tableAlias = null, $order = 20, $parameters = []): static
Copy link
Member Author

@GuySartorelli GuySartorelli Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This method wasn't strictly necessary, but it seems appropriate to reduce code duplication by moving the repeated code into this new method.
I can see a use case for this being public, but that's not the purpose of this PR. We can always do that as a separate future enhancement down the road.

{
if (!$tableAlias) {
$tableAlias = $table;
}
$this->from[$tableAlias] = [
'type' => 'INNER',
'type' => $type,
'table' => $table,
'filter' => [$onPredicate],
'order' => $order,
Expand Down
33 changes: 22 additions & 11 deletions tests/php/ORM/DataQueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,33 @@ public function testSortByJoinedFieldRetainsSourceInformation()
$this->assertEquals('Foo', $result['Title']);
}

public function provideJoins()
{
return [
[
'joinMethod' => 'innerJoin',
'joinType' => 'INNER',
],
[
'joinMethod' => 'leftJoin',
'joinType' => 'LEFT',
],
[
'joinMethod' => 'rightJoin',
'joinType' => 'RIGHT',
],
];
}

/**
* Test the leftJoin() and innerJoin method of the DataQuery object
* @dataProvider provideJoins
*/
public function testJoins()
public function testJoins($joinMethod, $joinType)
{
$dq = new DataQuery(Member::class);
$dq->innerJoin("Group_Members", "\"Group_Members\".\"MemberID\" = \"Member\".\"ID\"");
$this->assertSQLContains(
"INNER JOIN \"Group_Members\" ON \"Group_Members\".\"MemberID\" = \"Member\".\"ID\"",
$dq->sql($parameters)
);

$dq = new DataQuery(Member::class);
$dq->leftJoin("Group_Members", "\"Group_Members\".\"MemberID\" = \"Member\".\"ID\"");
$dq->$joinMethod("Group_Members", "\"Group_Members\".\"MemberID\" = \"Member\".\"ID\"");
$this->assertSQLContains(
"LEFT JOIN \"Group_Members\" ON \"Group_Members\".\"MemberID\" = \"Member\".\"ID\"",
"$joinType JOIN \"Group_Members\" ON \"Group_Members\".\"MemberID\" = \"Member\".\"ID\"",
$dq->sql($parameters)
);
}
Expand Down
57 changes: 28 additions & 29 deletions tests/php/ORM/SQLSelectTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,29 +435,33 @@ public function testFiltersOnFK()
);
}

public function testInnerJoin()
public function testJoinSQL()
Copy link
Member Author

Choose a reason for hiding this comment

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

This wasn't actually testing inner join - it was just testing the SQL output for both inner and left join. So I've just renamed it to match what it's actually testing, and added right join to it since it already had both existing join types in there.

{
$query = new SQLSelect();
$query->setFrom('MyTable');
$query->addInnerJoin('MyOtherTable', 'MyOtherTable.ID = 2');
$query->addRightJoin('MySecondTable', 'MyOtherTable.ID = MySecondTable.ID');
$query->addLeftJoin('MyLastTable', 'MyOtherTable.ID = MyLastTable.ID');

$this->assertSQLEquals(
'SELECT * FROM MyTable ' .
'INNER JOIN "MyOtherTable" ON MyOtherTable.ID = 2 ' .
'RIGHT JOIN "MySecondTable" ON MyOtherTable.ID = MySecondTable.ID ' .
'LEFT JOIN "MyLastTable" ON MyOtherTable.ID = MyLastTable.ID',
$query->sql($parameters)
);

$query = new SQLSelect();
$query->setFrom('MyTable');
$query->addInnerJoin('MyOtherTable', 'MyOtherTable.ID = 2', 'table1');
$query->addLeftJoin('MyLastTable', 'MyOtherTable.ID = MyLastTable.ID', 'table2');
$query->addRightJoin('MySecondTable', 'MyOtherTable.ID = MySecondTable.ID', 'table2');
$query->addLeftJoin('MyLastTable', 'MyOtherTable.ID = MyLastTable.ID', 'table3');

$this->assertSQLEquals(
'SELECT * FROM MyTable ' .
'INNER JOIN "MyOtherTable" AS "table1" ON MyOtherTable.ID = 2 ' .
'LEFT JOIN "MyLastTable" AS "table2" ON MyOtherTable.ID = MyLastTable.ID',
'RIGHT JOIN "MySecondTable" AS "table2" ON MyOtherTable.ID = MySecondTable.ID ' .
'LEFT JOIN "MyLastTable" AS "table3" ON MyOtherTable.ID = MyLastTable.ID',
$query->sql($parameters)
);
}
Expand Down Expand Up @@ -739,38 +743,33 @@ public function testLimitSetFromClauseString()
$this->assertEquals(10, $limit['start']);
}

public function testParameterisedInnerJoins()
public function provideParameterisedJoinSQL()
{
$query = new SQLSelect();
$query->setSelect(['"SQLSelectTest_DO"."Name"', '"SubSelect"."Count"']);
$query->setFrom('"SQLSelectTest_DO"');
$query->addInnerJoin(
'(SELECT "Title", COUNT(*) AS "Count" FROM "SQLSelectTestBase" GROUP BY "Title" HAVING "Title" NOT LIKE ?)',
'"SQLSelectTest_DO"."Name" = "SubSelect"."Title"',
'SubSelect',
20,
['%MyName%']
);
$query->addWhere(['"SQLSelectTest_DO"."Date" > ?' => '2012-08-08 12:00']);

$this->assertSQLEquals(
'SELECT "SQLSelectTest_DO"."Name", "SubSelect"."Count"
FROM "SQLSelectTest_DO" INNER JOIN (SELECT "Title", COUNT(*) AS "Count" FROM "SQLSelectTestBase"
GROUP BY "Title" HAVING "Title" NOT LIKE ?) AS "SubSelect" ON "SQLSelectTest_DO"."Name" =
"SubSelect"."Title"
WHERE ("SQLSelectTest_DO"."Date" > ?)',
$query->sql($parameters)
);
$this->assertEquals(['%MyName%', '2012-08-08 12:00'], $parameters);
$query->execute();
return [
[
'joinMethod' => 'addInnerJoin',
'joinType' => 'INNER',
],
[
'joinMethod' => 'addLeftJoin',
'joinType' => 'LEFT',
],
[
'joinMethod' => 'addRightJoin',
'joinType' => 'RIGHT',
],
];
}

public function testParameterisedLeftJoins()
/**
* @dataProvider provideParameterisedJoinSQL
*/
public function testParameterisedJoinSQL($joinMethod, $joinType)
{
$query = new SQLSelect();
$query->setSelect(['"SQLSelectTest_DO"."Name"', '"SubSelect"."Count"']);
$query->setFrom('"SQLSelectTest_DO"');
$query->addLeftJoin(
$query->$joinMethod(
'(SELECT "Title", COUNT(*) AS "Count" FROM "SQLSelectTestBase" GROUP BY "Title" HAVING "Title" NOT LIKE ?)',
'"SQLSelectTest_DO"."Name" = "SubSelect"."Title"',
'SubSelect',
Expand All @@ -781,7 +780,7 @@ public function testParameterisedLeftJoins()

$this->assertSQLEquals(
'SELECT "SQLSelectTest_DO"."Name", "SubSelect"."Count"
FROM "SQLSelectTest_DO" LEFT JOIN (SELECT "Title", COUNT(*) AS "Count" FROM "SQLSelectTestBase"
FROM "SQLSelectTest_DO" ' . $joinType . ' JOIN (SELECT "Title", COUNT(*) AS "Count" FROM "SQLSelectTestBase"
GROUP BY "Title" HAVING "Title" NOT LIKE ?) AS "SubSelect" ON "SQLSelectTest_DO"."Name" =
"SubSelect"."Title"
WHERE ("SQLSelectTest_DO"."Date" > ?)',
Expand Down