Skip to content

Commit

Permalink
Suport GROUP BY clause in datastore queries (#3731)
Browse files Browse the repository at this point in the history
  • Loading branch information
dafeder authored Jan 4, 2022
1 parent ded75fd commit f585071
Show file tree
Hide file tree
Showing 8 changed files with 244 additions and 9 deletions.
7 changes: 7 additions & 0 deletions modules/common/src/Storage/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ class Query implements
*/
public $joins = [];

/**
* Fields to group by in query.
*
* @var string[]
*/
public $groupby = [];

/**
* Limit for maximum number of records returned.
*
Expand Down
11 changes: 11 additions & 0 deletions modules/common/src/Storage/SelectFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public function create(Query $query): Select {

$this->setQueryProperties($query);
$this->setQueryConditions($query);
$this->setQueryGroupBy($query);
$this->setQueryOrderBy($query);
$this->setQueryLimitAndOffset($query);
$this->setQueryJoins($query);
Expand Down Expand Up @@ -283,6 +284,16 @@ private function addConditionGroup($statementObj, $conditionGroup) {
$statementObj->condition($group);
}

/**
* Set fields to group by on DB query.
*
* @param Query $query
* A DKAN query object.
*/
private function setQueryGroupBy(Query $query) {
array_map([$this->dbQuery, 'groupBy'], $query->groupby);
}

/**
* Set sort order on DB query.
*
Expand Down
52 changes: 52 additions & 0 deletions modules/common/tests/src/Unit/Storage/QueryDataProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public function getAllData($return): array {
'joinsQuery',
'joinWithPropertiesFromBothQuery',
'countQuery',
'groupByQuery',
];
$data = [];
foreach ($tests as $test) {
Expand Down Expand Up @@ -537,4 +538,55 @@ public static function countQuery($return) {
}
}

/**
* Provides an example groupby query object, SQL string, and exception string.
*
* @param int $returnType
* Expected groupby query return value type.
*
* @return Query|string
*/
public static function groupByQuery(int $returnType) {
switch ($returnType) {
case self::QUERY_OBJECT:
$query = new Query();
$query->properties = [
(object) [
'collection' => 't',
'property' => 'prop',
],
(object) [
'alias' => 'sum',
'expression' => (object) [
'operator' => 'sum',
'operands' => [
(object) [
'collection' => 't',
'property' => 'summable',
],
],
],
],
];
$query->conditions = [
(object) [
'collection' => 't',
'property' => 'filterable',
'value' => 'value',
'operator' => '=',
],
];
$query->groupby = ['prop'];
return $query;

case self::SQL:
return 'SELECT t.prop AS prop, (SUM(t.summable)) AS sum FROM {table} t WHERE t.filterable = :db_condition_placeholder_0 GROUP BY prop';

case self::EXCEPTION:
return '';
}

throw new \UnexpectedValueException('Unknown return type provided.');
}

}
12 changes: 11 additions & 1 deletion modules/datastore/docs/query.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@
"required": ["resource", "condition"]
}
},
"groupings": {
"type": "array",
"description": "Properties or aliases to group results by.",
"items": {
"anyOf": [
{ "$ref": "#/definitions/resource" },
{ "$ref": "#/definitions/resourceProperty" }
]
}
},
"limit": {
"type": "integer",
"description": "Limit for maximum number of records returned. Pass zero for no limit (may be restricted by user permissions)."
Expand Down Expand Up @@ -197,7 +207,7 @@
"properties": {
"operator": {
"type": "string",
"description": "Expression operator. Note that performing expressions on text or other non-numeric data types my yeild unexpected results.",
"description": "Expression operator. Note that performing expressions on text or other non-numeric data types my yield unexpected results.",
"enum": [
"+",
"-",
Expand Down
61 changes: 61 additions & 0 deletions modules/datastore/src/Storage/QueryFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public function populateQuery(): Query {
$this->populateQueryProperties($query);
$this->populateQueryConditions($query);
$this->populateQueryJoins($query);
$this->populateQueryGroupBy($query);
$this->populateQuerySorts($query);
if ($this->datastoreQuery->{"$.limit"}) {
$query->limit = $this->datastoreQuery->{"$.limit"};
Expand All @@ -75,6 +76,66 @@ public function populateQuery(): Query {
return $query;
}

/**
* Helper function for adding group by clauses to the given query.
*
* @param Drupal\common\Storage\Query $query
* DKAN query object we're building.
*
* @throws \Exception
* When ungrouped properties are found in the datastore query.
*/
private function populateQueryGroupBy(Query $query): void {
$groupings = $this->extractPropertyNames($this->datastoreQuery->{"$.groupings"} ?? []);
if (empty($groupings)) {
return;
}
$props = $this->extractPropertyNames($this->datastoreQuery->{"$.properties"} ?? []);
if ($ungrouped = array_diff($props, $groupings)) {
throw new \Exception('Un-grouped properties found in aggregate query: ' . implode(', ', $ungrouped));
}
$query->groupby = $groupings;
}

/**
* Extract list of property names from an array of properties.
*
* Properties can be either objects, associative-arrays, or strings. If a
* property is an object or associative-array, the property name is stored in
* the "property" field. If a property is a string, the string itself is a
* property name.
*
* @param array $props
* List of query properties (which can be a string, associative array, or
* object, see above).
*
* @return string[]
* List of extracted property names.
*
* @throws \Exception
* When an invaid property is encountered.
*/
protected function extractPropertyNames(array $props): array {
return array_filter(array_map(function ($prop) {
if (is_string($prop)) {
return $prop;
}
// If prop is an array, convert it to an object.
if (is_array($prop)) {
$prop = (object) $prop;
}
if (is_object($prop)) {
if (isset($prop->property)) {
return $prop->property;
}
// If this property object is an expression, ignore it.
if (isset($prop->expression)) {
return NULL;
}
}
}, $props));
}

/**
* Populate a query object with the queries from a datastore query payload.
*
Expand Down
40 changes: 40 additions & 0 deletions modules/datastore/tests/data/query/groupByQuery.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"resources": [
{
"id": "asdf",
"alias": "t"
}
],
"properties": [
{
"resource": "t",
"property": "prop"
},
{
"alias": "sum",
"expression": {
"operator": "sum",
"operands": [
{
"resource": "t",
"property": "summable"
}
]
}
}
],
"conditions": [
{
"resource": "t",
"property": "filterable",
"value": "value",
"operator": "="
}
],
"groupings": [
{
"resource": "t",
"property": "prop"
}
]
}
41 changes: 41 additions & 0 deletions modules/datastore/tests/data/query/ungroupedGroupByQuery.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"resources": [
{
"id": "asdf",
"alias": "t"
}
],
"properties": [
"prop",
{
"resource": "t",
"property": "prop2"
},
{
"alias": "sum",
"expression": {
"operator": "sum",
"operands": [
{
"resource": "t",
"property": "summable"
}
]
}
}
],
"conditions": [
{
"resource": "t",
"property": "filterable",
"value": "value",
"operator": "="
}
],
"groupings": [
{
"resource": "t",
"property": "prop"
}
]
}
29 changes: 21 additions & 8 deletions modules/datastore/tests/src/Unit/Service/DatastoreQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ public function testInvalidQueryAgainstSchema() {
$datastoreService->runQuery($datastoreQuery);
}

/**
* Ensure if an ungrouped property is specified, the query fails.
*/
public function testUngroupedPropertyInGroupByQueryFails(): void {
$this->expectExceptionMessage('Un-grouped properties found in aggregate query: prop2');
$container = $this->getCommonMockChain();
\Drupal::setContainer($container->getMock());
$datastoreService = Service::create($container->getMock());
$datastoreQuery = $this->getDatastoreQueryFromJson('ungroupedGroupByQuery');
$datastoreService->runQuery($datastoreQuery);
}

public function testRowIdsQuery() {
$container = $this->getCommonMockChain()
->add(DatabaseTable::class, "getSchema", [
Expand Down Expand Up @@ -150,14 +162,15 @@ public function testRowIdsQuery() {
*/
public function queryCompareProvider() {
return [
["propertiesQuery"],
["expressionQuery"],
["arrayConditionQuery"],
["likeConditionQuery"],
["nestedExpressionQuery"],
["nestedConditionGroupQuery"],
["sortQuery"],
["joinWithPropertiesFromBothQuery"],
['propertiesQuery'],
['expressionQuery'],
['arrayConditionQuery'],
['likeConditionQuery'],
['nestedExpressionQuery'],
['nestedConditionGroupQuery'],
['sortQuery'],
['joinWithPropertiesFromBothQuery'],
['groupByQuery'],
];
}

Expand Down

0 comments on commit f585071

Please sign in to comment.