Skip to content

Commit 29d9292

Browse files
committed
feat(database-adapter): validate groupBy queries, share contract with ClickHouse
The Database adapter does not push aggregation through utopia-php/database (no groupBy on the base Query class), so groupBy queries continue to be elided from the converted query list - callers get raw rows just as they do today with groupByInterval. The change is in validation: rejecting malformed groupBy use up front means the negative-case contract is identical on both backends. - validateGroupByQueries() runs at the top of convertQueriesToDatabase. Rejects groupBy attributes outside the union of EVENT_COLUMNS and GAUGE_COLUMNS (the Database adapter shares one collection for both types). Rejects groupBy without an accompanying groupByInterval. - Negative-case integration tests live in UsageBase so both adapters exercise them: unknown attribute throws, groupBy-without-interval throws.
1 parent 3d49192 commit 29d9292

2 files changed

Lines changed: 76 additions & 2 deletions

File tree

src/Usage/Adapter/Database.php

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,13 @@ public function sumDaily(array $queries = [], string $attribute = 'value'): int
367367
*
368368
* @param array<Query> $queries
369369
* @return array<DatabaseQuery>
370+
* @throws \Exception When a groupBy attribute is not a valid dimension column,
371+
* or when groupBy is used without groupByInterval.
370372
*/
371373
private function convertQueriesToDatabase(array $queries): array
372374
{
375+
$this->validateGroupByQueries($queries);
376+
373377
$dbQueries = [];
374378
foreach ($queries as $query) {
375379
$method = $query->getMethod();
@@ -474,15 +478,62 @@ private function convertQueriesToDatabase(array $queries): array
474478
break;
475479

476480
case UsageQuery::TYPE_GROUP_BY_INTERVAL:
477-
// groupByInterval is not supported by the Database adapter.
478-
// Silently skip — callers get raw (non-aggregated) results.
481+
case UsageQuery::TYPE_GROUP_BY:
482+
// groupByInterval and groupBy are not pushed down to the
483+
// Database adapter; callers get raw (non-aggregated) results.
484+
// Validation runs in validateGroupByQueries() before this loop.
479485
break;
480486
}
481487
}
482488

483489
return $dbQueries;
484490
}
485491

492+
/**
493+
* Validate groupBy / groupByInterval interactions in the supplied queries.
494+
*
495+
* Mirrors the ClickHouse adapter contract: groupBy attributes must exist on
496+
* the matching schema (event vs gauge — we default to the broader event set
497+
* for the Database adapter since both share one collection), and groupBy
498+
* must always be paired with groupByInterval so the cloud-facing API stays
499+
* consistent across backends.
500+
*
501+
* @param array<Query> $queries
502+
* @throws \Exception
503+
*/
504+
private function validateGroupByQueries(array $queries): void
505+
{
506+
$hasGroupBy = false;
507+
$hasGroupByInterval = false;
508+
$allowed = array_unique(array_merge(Metric::EVENT_COLUMNS, Metric::GAUGE_COLUMNS));
509+
510+
foreach ($queries as $query) {
511+
$method = $query->getMethod();
512+
513+
if ($method === UsageQuery::TYPE_GROUP_BY_INTERVAL) {
514+
$hasGroupByInterval = true;
515+
continue;
516+
}
517+
518+
if ($method !== UsageQuery::TYPE_GROUP_BY) {
519+
continue;
520+
}
521+
522+
$hasGroupBy = true;
523+
$attribute = $query->getAttribute();
524+
525+
if (!in_array($attribute, $allowed, true)) {
526+
throw new \Exception(
527+
"Invalid groupBy attribute '{$attribute}'. Allowed: " . implode(', ', $allowed)
528+
);
529+
}
530+
}
531+
532+
if ($hasGroupBy && !$hasGroupByInterval) {
533+
throw new \Exception('groupBy requires groupByInterval to be specified');
534+
}
535+
}
536+
486537
/**
487538
* @param array<Query> $queries
488539
* @param string|null $type

tests/Usage/UsageBase.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,4 +639,27 @@ public function testGroupByIntervalWithLimitOffset(): void
639639

640640
$this->assertGreaterThanOrEqual(1, count($results));
641641
}
642+
643+
public function testGroupByUnknownAttributeThrows(): void
644+
{
645+
$this->expectException(\Exception::class);
646+
$this->expectExceptionMessageMatches("/groupBy attribute 'not_a_column'/");
647+
648+
$this->usage->find([
649+
UsageQuery::groupByInterval('time', '1h'),
650+
UsageQuery::groupBy('not_a_column'),
651+
Query::equal('metric', ['gbi-requests']),
652+
], Usage::TYPE_EVENT);
653+
}
654+
655+
public function testGroupByWithoutGroupByIntervalThrows(): void
656+
{
657+
$this->expectException(\Exception::class);
658+
$this->expectExceptionMessageMatches('/groupBy requires groupByInterval/');
659+
660+
$this->usage->find([
661+
UsageQuery::groupBy('service'),
662+
Query::equal('metric', ['gbi-requests']),
663+
], Usage::TYPE_EVENT);
664+
}
642665
}

0 commit comments

Comments
 (0)