Skip to content

Commit

Permalink
FIX Use OR conjuctive in filterAny aggregate queries
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz committed May 16, 2023
1 parent 7210ac8 commit 505e661
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 18 deletions.
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
20 changes: 18 additions & 2 deletions src/ORM/DataQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -662,9 +662,18 @@ public function having($having)
*/
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
*
Expand All @@ -674,7 +683,14 @@ public function disjunctiveGroup()
*/
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
57 changes: 45 additions & 12 deletions src/ORM/DataQuery_SubGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,39 @@
*/
class DataQuery_SubGroup extends DataQuery implements SQLConditionGroup
{
private string $clause;

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

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

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);
}
}

public function where($filter)
Expand All @@ -49,18 +67,33 @@ public function whereAny($filter)
return $this;
}

public function having($filter)
{
if ($filter) {
$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
28 changes: 28 additions & 0 deletions tests/php/ORM/DataListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,34 @@ 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));
$this->assertSame([$team1->ID, $team3->ID], $list->column('ID'));
}

public function testFilterOnJoin()
{
$list = TeamComment::get()
Expand Down

0 comments on commit 505e661

Please sign in to comment.