feat(clickhouse): support groupBy dimension columns in aggregated find

lohanidamodar · lohanidamodar · commit 3d49192c9621 · 2026-05-27T13:22:15.000Z
Extends findAggregatedFromTable so callers can bucket aggregated rows by
arbitrary dimension columns (service, path, status, ...) alongside the
existing time bucket.

- parseQueries() collects groupBy attributes into parsed[groupBy] and
  validates each against the type-specific column set
  (Metric::EVENT_COLUMNS / Metric::GAUGE_COLUMNS) via the new
  validateGroupByAttribute() helper. Unknown columns raise a
  descriptive Exception listing what is allowed.
- findFromTable() rejects groupBy without groupByInterval - keeps the
  contract simple (caller always supplies a time bucket too) and matches
  the Database adapter behaviour.
- findAggregatedFromTable() appends the escaped dim columns to both
  SELECT and GROUP BY. parseAggregatedResults() already passes unknown
  keys through to the Metric so the dim values surface as plain getters
  (getService, getPath, ...).
- Integration tests: single-dim groupBy(service) over 1d aggregates to
  the expected per-service sums; multi-dim groupBy(service)+groupBy(path)
  over 1h produces the cartesian grouping with correct sums per pair.
diff --git a/src/Usage/Adapter/ClickHouse.php b/src/Usage/Adapter/ClickHouse.php
@@ -1179,6 +1179,27 @@ private function validateAttributeName(string $attributeName, string $type = 'ev
         throw new Exception("Invalid attribute name: {$attributeName}");
     }
 
+    /**
+     * Validate that a groupBy attribute is an aggregable dimension column.
+     *
+     * Restricted to the indexed dimension columns for the table type — `metric`,
+     * `value` and `time` are excluded since they are already part of the
+     * aggregation (metric is in the SELECT, time is bucketed via
+     * groupByInterval, value is the measured quantity).
+     *
+     * @throws Exception
+     */
+    private function validateGroupByAttribute(string $attribute, string $type): bool
+    {
+        $allowed = $type === Usage::TYPE_GAUGE ? Metric::GAUGE_COLUMNS : Metric::EVENT_COLUMNS;
+
+        if (in_array($attribute, $allowed, true)) {
+            return true;
+        }
+
+        throw new Exception("Invalid groupBy attribute '{$attribute}' for {$type}. Allowed: " . implode(', ', $allowed));
+    }
+
     /**
      * Columns available in the events daily (pre-aggregated) table.
      */
@@ -1567,6 +1588,10 @@ private function findFromTable(array $queries, string $type): array
             throw new Exception('Cursor pagination cannot be combined with groupByInterval');
         }
 
+        if (!empty($parsed['groupBy']) && !isset($parsed['groupByInterval'])) {
+            throw new Exception('groupBy requires groupByInterval to be specified');
+        }
+
         // Check if groupByInterval is requested
         if (isset($parsed['groupByInterval'])) {
             return $this->findAggregatedFromTable($parsed, $fromTable, $type);
@@ -1629,7 +1654,7 @@ private function findFromTable(array $queries, string $type): array
      *          toStartOfInterval(time, INTERVAL 1 HOUR) as time
      *   FROM table WHERE ... GROUP BY metric, time ORDER BY time ASC
      *
-     * @param array{filters: array<string>, params: array<string, mixed>, orderBy?: array<string>, limit?: int, offset?: int, groupByInterval?: string} $parsed Parsed query data from parseQueries()
+     * @param array{filters: array<string>, params: array<string, mixed>, orderBy?: array<string>, limit?: int, offset?: int, groupByInterval?: string, groupBy?: array<int, string>} $parsed Parsed query data from parseQueries()
      * @param string $fromTable Fully qualified table reference
      * @param string $type 'event' or 'gauge'
      * @return array<Metric>
@@ -1650,6 +1675,18 @@ private function findAggregatedFromTable(array $parsed, string $fromTable, strin
         // then alias back to 'time' in outer context for consistent Metric parsing.
         $timeBucketExpr = "toStartOfInterval(time, {$intervalSql})";
 
+        $groupByDims = $parsed['groupBy'] ?? [];
+        $dimSelect = '';
+        $dimGroup = '';
+        if (!empty($groupByDims)) {
+            $escapedDims = array_map(
+                fn (string $dim): string => $this->escapeIdentifier($dim),
+                $groupByDims
+            );
+            $dimSelect = ', ' . implode(', ', $escapedDims);
+            $dimGroup = ', ' . implode(', ', $escapedDims);
+        }
+
         $whereData = $this->buildWhereClause($parsed['filters'], $parsed['params']);
         $whereClause = $whereData['clause'];
         $params = $whereData['params'];
@@ -1676,9 +1713,9 @@ private function findAggregatedFromTable(array $parsed, string $fromTable, strin
         $offsetClause = isset($parsed['offset']) ? ' OFFSET {offset:UInt64}' : '';
 
         $sql = "
-            SELECT metric, {$valueExpr}, {$timeBucketExpr} as bucket
+            SELECT metric, {$valueExpr}, {$timeBucketExpr} as bucket{$dimSelect}
             FROM {$fromTable}{$whereClause}
-            GROUP BY metric, bucket{$orderClause}{$limitClause}{$offsetClause}
+            GROUP BY metric, bucket{$dimGroup}{$orderClause}{$limitClause}{$offsetClause}
             FORMAT JSON
         ";
 
@@ -2764,7 +2801,7 @@ private function buildOrderBySql(array $orderAttributes, bool $flip = false): ar
      *
      * @param array<Query> $queries
      * @param string $type 'event' or 'gauge' — used for attribute validation
-     * @return array{filters: array<string>, params: array<string, mixed>, orderBy?: array<string>, orderAttributes?: array<int, array{attribute: string, direction: string}>, limit?: int, offset?: int, groupByInterval?: string, cursor?: array<string, mixed>, cursorDirection?: string}
+     * @return array{filters: array<string>, params: array<string, mixed>, orderBy?: array<string>, orderAttributes?: array<int, array{attribute: string, direction: string}>, limit?: int, offset?: int, groupByInterval?: string, groupBy?: array<int, string>, cursor?: array<string, mixed>, cursorDirection?: string}
      * @throws Exception
      */
     private function parseQueries(array $queries, string $type = 'event'): array
@@ -2776,6 +2813,7 @@ private function parseQueries(array $queries, string $type = 'event'): array
         $limit = null;
         $offset = null;
         $groupByInterval = null;
+        $groupBy = [];
         $cursor = null;
         $cursorDirection = null;
         $paramCounter = 0;
@@ -3011,6 +3049,13 @@ private function parseQueries(array $queries, string $type = 'event'): array
                     }
                     $groupByInterval = $interval;
                     break;
+
+                case UsageQuery::TYPE_GROUP_BY:
+                    $this->validateGroupByAttribute($attribute, $type);
+                    if (!in_array($attribute, $groupBy, true)) {
+                        $groupBy[] = $attribute;
+                    }
+                    break;
             }
         }
 
@@ -3036,6 +3081,10 @@ private function parseQueries(array $queries, string $type = 'event'): array
             $result['groupByInterval'] = $groupByInterval;
         }
 
+        if (!empty($groupBy)) {
+            $result['groupBy'] = $groupBy;
+        }
+
         if ($cursor !== null && $cursorDirection !== null) {
             $result['cursor'] = $cursor;
             $result['cursorDirection'] = $cursorDirection;
diff --git a/tests/Usage/Adapter/ClickHouseTest.php b/tests/Usage/Adapter/ClickHouseTest.php
@@ -1241,6 +1241,79 @@ public function testCursorWithGroupByIntervalThrows(): void
         ], Usage::TYPE_EVENT);
     }
 
+    public function testGroupByServiceDailyAggregates(): void
+    {
+        $this->usage->purge();
+
+        $this->assertTrue($this->usage->addBatch([
+            ['metric' => 'gb-service', 'value' => 10, 'tags' => ['service' => 'storage']],
+            ['metric' => 'gb-service', 'value' => 25, 'tags' => ['service' => 'storage']],
+            ['metric' => 'gb-service', 'value' => 5, 'tags' => ['service' => 'databases']],
+        ], Usage::TYPE_EVENT));
+
+        $start = (new \DateTime())->modify('-1 day')->format('Y-m-d\TH:i:s');
+        $end = (new \DateTime())->modify('+1 day')->format('Y-m-d\TH:i:s');
+
+        $results = $this->usage->find([
+            UsageQuery::groupByInterval('time', '1d'),
+            UsageQuery::groupBy('service'),
+            Query::equal('metric', ['gb-service']),
+            Query::greaterThanEqual('time', $start),
+            Query::lessThanEqual('time', $end),
+        ], Usage::TYPE_EVENT);
+
+        $this->assertGreaterThanOrEqual(2, count($results));
+
+        $byService = [];
+        foreach ($results as $row) {
+            $service = $row->getService();
+            $this->assertNotNull($service, 'groupBy(service) should surface the service dimension on each Metric');
+            $byService[$service] = ($byService[$service] ?? 0) + $row->getValue();
+        }
+
+        $this->assertEquals(35, $byService['storage'] ?? null);
+        $this->assertEquals(5, $byService['databases'] ?? null);
+    }
+
+    public function testGroupByMultipleDimensionsHourly(): void
+    {
+        $this->usage->purge();
+
+        $this->assertTrue($this->usage->addBatch([
+            ['metric' => 'gb-multi', 'value' => 1, 'tags' => ['service' => 'storage', 'path' => '/v1/a']],
+            ['metric' => 'gb-multi', 'value' => 2, 'tags' => ['service' => 'storage', 'path' => '/v1/a']],
+            ['metric' => 'gb-multi', 'value' => 4, 'tags' => ['service' => 'storage', 'path' => '/v1/b']],
+            ['metric' => 'gb-multi', 'value' => 8, 'tags' => ['service' => 'databases', 'path' => '/v1/a']],
+        ], Usage::TYPE_EVENT));
+
+        $start = (new \DateTime())->modify('-1 hour')->format('Y-m-d\TH:i:s');
+        $end = (new \DateTime())->modify('+1 hour')->format('Y-m-d\TH:i:s');
+
+        $results = $this->usage->find([
+            UsageQuery::groupByInterval('time', '1h'),
+            UsageQuery::groupBy('service'),
+            UsageQuery::groupBy('path'),
+            Query::equal('metric', ['gb-multi']),
+            Query::greaterThanEqual('time', $start),
+            Query::lessThanEqual('time', $end),
+        ], Usage::TYPE_EVENT);
+
+        $this->assertGreaterThanOrEqual(3, count($results));
+
+        $byPair = [];
+        foreach ($results as $row) {
+            $svc = $row->getService();
+            $path = $row->getPath();
+            $this->assertNotNull($svc);
+            $this->assertNotNull($path);
+            $byPair["{$svc}|{$path}"] = ($byPair["{$svc}|{$path}"] ?? 0) + $row->getValue();
+        }
+
+        $this->assertEquals(3, $byPair['storage|/v1/a'] ?? null);
+        $this->assertEquals(4, $byPair['storage|/v1/b'] ?? null);
+        $this->assertEquals(8, $byPair['databases|/v1/a'] ?? null);
+    }
+
     public function testNotEqualQuery(): void
     {
         // Fixture: requests x2, bandwidth x1 in events
