FIX Use OR conjuctive in filterAny aggregate queries

This commit is contained in:
Steve Boyd 2023-05-17 11:55:56 +12:00
parent 7210ac8cd3
commit 696fe79dd6
6 changed files with 177 additions and 20 deletions

View File

@ -12,6 +12,7 @@ use InvalidArgumentException;
use LogicException;
use BadMethodCallException;
use Traversable;
use SilverStripe\ORM\DataQuery;
/**
* Implements a "lazy loading" DataObjectSet.
@ -525,14 +526,27 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
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);
}
/**

View File

@ -658,23 +658,41 @@ class DataQuery
*
* 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);
}
/**

View File

@ -4,6 +4,8 @@ namespace SilverStripe\ORM;
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}
@ -14,26 +16,54 @@ use SilverStripe\ORM\Queries\SQLSelect;
*/
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);
}
@ -42,25 +72,46 @@ class DataQuery_SubGroup extends DataQuery implements SQLConditionGroup
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;
}
}

View File

@ -15,7 +15,6 @@ use Exception;
*/
class ManyManyList extends RelationList
{
/**
* @var string $joinTable
*/

View File

@ -1073,6 +1073,36 @@ class DataListTest extends SapphireTest
);
}
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()

View File

@ -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([]);
}
}