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 Use OR conjuctive in filterAny aggregate queries #10778

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 17 additions & 3 deletions src/ORM/DataList.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use LogicException;
use BadMethodCallException;
use Traversable;
use SilverStripe\ORM\DataQuery;

/**
* Implements a "lazy loading" DataObjectSet.
Expand Down Expand Up @@ -525,14 +526,27 @@ public function filterAny()
throw new InvalidArgumentException('Incorrect number of arguments passed to filterAny()');
}

return $this->alterDataQuery(function (DataQuery $query) use ($whereArguments) {
$subquery = $query->disjunctiveGroup();

$list = $this->alterDataQuery(function (DataQuery $query) use ($whereArguments) {
$subquery = $this->getFilterAnySubquery($query, $whereArguments);
foreach ($whereArguments as $field => $value) {
$filter = $this->createSearchFilter($field, $value);
$filter->apply($subquery);
}
});

return $list;
}

private function getFilterAnySubquery(DataQuery $query, array $whereArguments): DataQuery_SubGroup
{
$clause = 'WHERE';
foreach (array_keys($whereArguments) as $field) {
if (preg_match('#\.(COUNT|SUM|AVG|MIN|MAX)\(#', strtoupper($field))) {
$clause = 'HAVING';
break;
}
}
return $query->disjunctiveGroup($clause);
}

/**
Expand Down
22 changes: 20 additions & 2 deletions src/ORM/DataQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -658,23 +658,41 @@ public function having($having)
*
* That is a subgroup joined by OR
*
* @param string $clause
* @return DataQuery_SubGroup
*/
public function disjunctiveGroup()
{
return new DataQuery_SubGroup($this, 'OR');
// using func_get_args to add a new param while retaining BC
// @deprecated - add a new param for CMS 6 - string $clause = 'WHERE'
$clause = 'WHERE';
$args = func_get_args();
if (count($args) > 0) {
$clause = $args[0];
}
return new DataQuery_SubGroup($this, 'OR', $clause);
}



/**
* Create a conjunctive subgroup
*
* That is a subgroup joined by AND
*
* @param string $clause
* @return DataQuery_SubGroup
*/
public function conjunctiveGroup()
{
return new DataQuery_SubGroup($this, 'AND');
// using func_get_args to add a new param while retaining BC
// @deprecated - add a new param for CMS 6 - string $clause = 'WHERE'
$clause = 'WHERE';
$args = func_get_args();
if (count($args) > 0) {
$clause = $args[0];
}
return new DataQuery_SubGroup($this, 'AND', $clause);
}

/**
Expand Down
79 changes: 65 additions & 14 deletions src/ORM/DataQuery_SubGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use SilverStripe\ORM\Queries\SQLConditionGroup;
use SilverStripe\ORM\Queries\SQLSelect;
use InvalidArgumentException;
use LogicException;

/**
* Represents a subgroup inside a WHERE clause in a {@link DataQuery}
Expand All @@ -14,26 +16,54 @@
*/
class DataQuery_SubGroup extends DataQuery implements SQLConditionGroup
{
private string $clause;

/**
*
* @var SQLSelect
*/
protected $whereQuery;

/**
* @var SQLSelect
*/
protected $havingQuery;

/**
* @param DataQuery $base
* @param string $connective
* @param string $clause
*/
public function __construct(DataQuery $base, $connective)
{
// using func_get_args to add a 3rd param while retaining BC
// @deprecated - add a 3rd param for CMS 6 - string $clause = 'WHERE'
$clause = 'WHERE';
$args = func_get_args();
if (count($args) > 2) {
$clause = $args[2];
}
parent::__construct($base->dataClass);
$this->query = $base->query;
$this->whereQuery = new SQLSelect();
$this->whereQuery->setConnective($connective);

$base->where($this);
$this->clause = strtoupper($clause);
if ($this->clause === 'WHERE') {
$this->whereQuery = new SQLSelect();
$this->whereQuery->setConnective($connective);
$base->where($this);
} elseif ($this->clause === 'HAVING') {
$this->havingQuery = new SQLSelect();
$this->havingQuery->setConnective($connective);
$base->having($this);
} else {
throw new InvalidArgumentException('$clause must be either WHERE or HAVING');
}
}

public function where($filter)
{
if ($filter) {
if ($this->clause === 'HAVING') {
throw new LogicException('Cannot call where() when clause is set to HAVING');
}
if ($filter && $this->whereQuery) {
$this->whereQuery->addWhere($filter);
}

Expand All @@ -42,25 +72,46 @@ public function where($filter)

public function whereAny($filter)
{
if ($filter) {
if ($this->clause === 'HAVING') {
throw new LogicException('Cannot call whereAny() when clause is set to HAVING');
}
if ($filter && $this->whereQuery) {
$this->whereQuery->addWhereAny($filter);
}

return $this;
}

public function having($filter)
{
if ($this->clause === 'WHERE') {
throw new LogicException('Cannot call having() when clause is set to WHERE');
}
if ($filter && $this->havingQuery) {
$this->havingQuery->addHaving($filter);
}

return $this;
}

public function conditionSQL(&$parameters)
{
$parameters = [];

// Ignore empty conditions
$where = $this->whereQuery->getWhere();
if (empty($where)) {
return null;
if ($this->clause === 'WHERE') {
$where = $this->whereQuery->getWhere();
if (!empty($where)) {
$sql = DB::get_conn()->getQueryBuilder()->buildWhereFragment($this->whereQuery, $parameters);
return preg_replace('/^\s*WHERE\s*/i', '', $sql ?? '');
}
} elseif ($this->clause === 'HAVING') {
$having = $this->havingQuery->getHaving();
if (!empty($having)) {
$sql = DB::get_conn()->getQueryBuilder()->buildHavingFragment($this->havingQuery, $parameters);
return preg_replace('/^\s*HAVING\s*/i', '', $sql ?? '');
}
}

// Allow database to manage joining of conditions
$sql = DB::get_conn()->getQueryBuilder()->buildWhereFragment($this->whereQuery, $parameters);
return preg_replace('/^\s*WHERE\s*/i', '', $sql ?? '');
return null;
}
}
1 change: 0 additions & 1 deletion src/ORM/ManyManyList.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
class ManyManyList extends RelationList
{

/**
* @var string $joinTable
*/
Expand Down
30 changes: 30 additions & 0 deletions tests/php/ORM/DataListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,36 @@ public function testFilterAnyArrayInArray()
);
}

private function createTeam(int $playerCount)
{
$team = Team::create();
$team->write();
for ($i = 0; $i < $playerCount; $i++) {
$player = Player::create();
$player->write();
$team->Players()->add($player);
}
return $team;
}

public function testFilterAnyManyManyAggregate()
{
Team::get()->removeAll();
$team1 = $this->createTeam(1);
$team2 = $this->createTeam(2);
$team3 = $this->createTeam(3);
$list = Team::get()->filterAny([
'Players.Count():LessThan' => 2,
'Players.Count():GreaterThan' => 2,
]);
$match = 'HAVING ((COUNT("players_Member"."ID") < ?) OR (COUNT("players_Member"."ID") > ?))';
$sql = str_replace("\n", '', $list->sql());
$this->assertTrue(str_contains($sql, $match));
$ids = $list->column('ID');
sort($ids);
$this->assertSame([$team1->ID, $team3->ID], $ids);
}

public function testFilterOnJoin()
{
$list = TeamComment::get()
Expand Down
45 changes: 45 additions & 0 deletions tests/php/ORM/DataQuery_SubGroupTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace SilverStripe\ORM\Tests;

use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataQuery;
use SilverStripe\ORM\DataQuery_SubGroup;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Tests\DataObjectTest\Team;
use LogicException;
use InvalidArgumentException;

class DataQuery_SubGroupTest extends SapphireTest
{
public function testConstructorException()
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('$clause must be either WHERE or HAVING');
new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'INVALID');
}

public function testWhereException()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot call where() when clause is set to HAVING');
$query = new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'HAVING');
$query->where([]);
}

public function testWhereAnyException()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot call whereAny() when clause is set to HAVING');
$query = new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'HAVING');
$query->whereAny([]);
}

public function testHavingException()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot call having() when clause is set to WHERE');
$query = new DataQuery_SubGroup(new DataQuery(Team::class), 'AND', 'WHERE');
$query->having([]);
}
}