Skip to content

Commit 958596b

Browse files
committed
fix(adapter): exclude end-day row from daily route for inclusive-midnight upper bounds
Daily MV rows live at toStartOfDay(time). An inclusive `time <= midnight` predicate matched the row at that midnight, which represents the entire day starting at it — so callers received totals that over-included the end day's data. Two changes: - routedSum() translates inclusive `LESSER_EQUAL` time bounds at midnight to exclusive `LESSER` before delegating to sumDaily(). BETWEEN with an inclusive midnight upper is split into a `>=` lower + `<` upper pair. - buildDailyTimeFilters() (hybrid path) applies the same translation when emitting the daily-branch WHERE clause. Bench bounds in EventsBench were also relying on current time-of-day, which prevented bench_events_sum_30d from satisfying the day-aligned check and routed it to raw despite the route assertion expecting daily. Floored to UTC midnight so the scenario exercises the path it asserts. Regression test: seeds an event mid-day on the end day, queries with inclusive-midnight upper, asserts the daily-routed sum equals the raw sum (and does not include the late event).
1 parent 30073c3 commit 958596b

3 files changed

Lines changed: 89 additions & 5 deletions

File tree

src/Usage/Adapter/ClickHouse.php

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2155,7 +2155,7 @@ private function routedSum(array $queries, string $operation): int
21552155
$this->recordRoute($operation, $plan, $route);
21562156

21572157
if ($route === 'daily') {
2158-
$total = $this->sumDaily($queries, 'value');
2158+
$total = $this->sumDaily($this->translateInclusiveMidnightForDaily($queries), 'value');
21592159
$this->maybeDualRead($queries, $route, $plan, $total);
21602160
return $total;
21612161
}
@@ -2334,6 +2334,64 @@ private function isDayAligned(?DateTime $dt): bool
23342334
return $dt->format('H:i:s.u') === '00:00:00.000000';
23352335
}
23362336

2337+
private function isMidnightString(?string $ts): bool
2338+
{
2339+
if ($ts === null) {
2340+
return false;
2341+
}
2342+
try {
2343+
return $this->isDayAligned(new DateTime($ts));
2344+
} catch (Exception $e) {
2345+
return false;
2346+
}
2347+
}
2348+
2349+
/**
2350+
* Daily MV rows are keyed at toStartOfDay(time), so an inclusive
2351+
* `<= midnight` upper bound matches the row representing the entire
2352+
* end day and over-counts. Rewrite inclusive-midnight upper bounds
2353+
* (LESSER_EQUAL and BETWEEN upper) to exclusive `<` for the daily
2354+
* branch. Other bounds pass through untouched.
2355+
*
2356+
* @param array<Query> $queries
2357+
* @return array<Query>
2358+
*/
2359+
private function translateInclusiveMidnightForDaily(array $queries): array
2360+
{
2361+
$result = [];
2362+
foreach ($queries as $q) {
2363+
if (!($q instanceof Query) || $q->getAttribute() !== 'time') {
2364+
$result[] = $q;
2365+
continue;
2366+
}
2367+
$method = $q->getMethod();
2368+
$values = $q->getValues();
2369+
2370+
if ($method === Query::TYPE_LESSER_EQUAL) {
2371+
$upper = $this->stringifyTime($values[0] ?? null);
2372+
if ($upper !== null && $this->isMidnightString($upper)) {
2373+
$result[] = new Query(Query::TYPE_LESSER, 'time', [$upper]);
2374+
continue;
2375+
}
2376+
}
2377+
2378+
if ($method === Query::TYPE_BETWEEN && count($values) >= 2) {
2379+
$upper = $this->stringifyTime($values[1] ?? null);
2380+
if ($upper !== null && $this->isMidnightString($upper)) {
2381+
$lower = $this->stringifyTime($values[0] ?? null);
2382+
if ($lower !== null) {
2383+
$result[] = new Query(Query::TYPE_GREATER_EQUAL, 'time', [$lower]);
2384+
}
2385+
$result[] = new Query(Query::TYPE_LESSER, 'time', [$upper]);
2386+
continue;
2387+
}
2388+
}
2389+
2390+
$result[] = $q;
2391+
}
2392+
return $result;
2393+
}
2394+
23372395
private function stringifyTime(mixed $value): ?string
23382396
{
23392397
if ($value instanceof DateTime) {
@@ -2550,7 +2608,8 @@ private function buildDailyTimeFilters(array $existing, array $timeQueries, arra
25502608
}
25512609
$name = 'daily_time_upper_' . $counter++;
25522610
$params[$name] = $upper;
2553-
$op = $method === Query::TYPE_LESSER ? '<' : '<=';
2611+
$inclusiveOnMidnight = $method === Query::TYPE_LESSER_EQUAL && $this->isMidnightString($upper);
2612+
$op = ($method === Query::TYPE_LESSER || $inclusiveOnMidnight) ? '<' : '<=';
25542613
$filters[] = '`time` ' . $op . ' {' . $name . ':DateTime64(3, \'UTC\')}';
25552614
continue;
25562615
}
@@ -2566,7 +2625,8 @@ private function buildDailyTimeFilters(array $existing, array $timeQueries, arra
25662625
if ($upper !== null) {
25672626
$name = 'daily_time_upper_' . $counter++;
25682627
$params[$name] = $upper;
2569-
$filters[] = '`time` <= {' . $name . ':DateTime64(3, \'UTC\')}';
2628+
$op = $this->isMidnightString($upper) ? '<' : '<=';
2629+
$filters[] = '`time` ' . $op . ' {' . $name . ':DateTime64(3, \'UTC\')}';
25702630
}
25712631
continue;
25722632
}

tests/Benchmark/EventsBench.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ protected function setUp(): void
2020

2121
public function testBenchmarks(): void
2222
{
23-
$start = (new DateTime('-30 days'))->format('Y-m-d H:i:s');
24-
$end = (new DateTime('-1 day'))->format('Y-m-d H:i:s');
23+
$start = (new DateTime('-30 days', new DateTimeZone('UTC')))->setTime(0, 0, 0)->format('Y-m-d H:i:s');
24+
$end = (new DateTime('-1 day', new DateTimeZone('UTC')))->setTime(0, 0, 0)->format('Y-m-d H:i:s');
2525

2626
$this->runBench('bench_events_sum_30d', function (string $queryId) use ($start, $end): void {
2727
$this->adapter->setNextQueryId($queryId);

tests/Usage/Adapter/ClickHouseRoutingTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,30 @@ public function testClosedDayWindowRoutesToDaily(): void
8686
$this->assertSame($rawSum, $sum, 'daily MV must re-aggregate to the same total as raw');
8787
}
8888

89+
public function testInclusiveMidnightUpperBoundExcludesEndDayOnDailyRoute(): void
90+
{
91+
$this->adapter->clearRouteLog();
92+
93+
$this->seedHistoricalRow('routed.metric', 9999, '-2 days +14 hours', ['path' => '/v1/late']);
94+
95+
$start = (new DateTime('-7 days', new DateTimeZone('UTC')))->setTime(0, 0, 0)->format('Y-m-d H:i:s');
96+
$end = (new DateTime('-2 days', new DateTimeZone('UTC')))->setTime(0, 0, 0)->format('Y-m-d H:i:s');
97+
98+
$rawSum = $this->sumRaw('routed.metric', $start, $end);
99+
100+
$sum = $this->usage->sum([
101+
Query::equal('metric', ['routed.metric']),
102+
Query::greaterThanEqual('time', $start),
103+
Query::lessThanEqual('time', $end),
104+
], 'value', Usage::TYPE_EVENT);
105+
106+
$log = $this->adapter->getRouteLog();
107+
$this->assertCount(1, $log);
108+
$this->assertSame('daily', $log[0]['route']);
109+
$this->assertSame($rawSum, $sum, 'daily MV must not include the end-day full-day row for inclusive-midnight upper bounds');
110+
$this->assertSame(300, $sum);
111+
}
112+
89113
public function testMidDayClosedWindowFallsBackToRaw(): void
90114
{
91115
// Daily rows are stored at midnight; a mid-day caller bound

0 commit comments

Comments
 (0)