Skip to content

feat(clickhouse): optional retention TTL on the raw events table#24

Merged
loks0n merged 2 commits into
utopia-php:mainfrom
levivannoort:feat/retention-ttl-main
Jul 8, 2026
Merged

feat(clickhouse): optional retention TTL on the raw events table#24
loks0n merged 2 commits into
utopia-php:mainfrom
levivannoort:feat/retention-ttl-main

Conversation

@levivannoort

Copy link
Copy Markdown
Contributor

Ports the retention-TTL feature onto main directly (the branches have diverged, so the earlier dev→main PR #23 doesn't apply — this supersedes it).

What

Adds an optional retention window (days) to the ClickHouse adapter. When set, setup() applies an idempotent TTL to the raw events table so rows older than the window are dropped by background merges. The aggregated events_daily table — which backs long-term usage/billing history — is left untouched. Null (the default) disables retention and actively strips any TTL a previous run applied, keeping setup() re-runnable.

Style note

Exposed as a readonly constructor property rather than a getter/setter, matching the adapter's other config fields — more coroutine-safe (no post-construction mutation).

Verification

  • composer check (phpstan): clean
  • vendor/bin/pint --test: pass
  • Constructor validation (rejects non-positive days) verified standalone
  • TTL DDL assertion added to ClickHouseSchemaTest (integration-gated on a live ClickHouse, like the rest of that class)

🤖 Generated with Claude Code

Adds a readonly `retention` constructor property (days). When set, setup()
applies an idempotent TTL to the raw events table so rows older than the
window are dropped by background merges; the aggregated events_daily table
that backs long-term usage/billing history is left untouched. Null (default)
disables retention and actively strips any TTL a previous run applied.

Uses a readonly constructor property rather than a getter/setter for
coroutine safety, matching the adapter's other config fields.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds an optional retention window (days) to the ClickHouse adapter. When set, setup() runs an idempotent ALTER TABLE … MODIFY TTL on the raw events table; when null, it issues REMOVE TTL and swallows the ClickHouse error that fires when no TTL was previously set.

  • src/Usage/Adapter/ClickHouse.php: new ?int $retention constructor parameter validated ≥ 1; applyEventsRetention() called inside setup() after the events table is created and before the daily table; error-swallowing dual-anchors on Code: 36 + TTL to avoid masking unrelated BAD_ARGUMENTS failures; the events_daily table is intentionally left untouched.
  • tests/Usage/Adapter/ClickHouseSchemaTest.php: integration test asserts the DDL contains the TTL clause on the events table and no TTL on the daily table; a unit-level test exercises the constructor guard for non-positive retention values without requiring a live ClickHouse instance.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to a new optional parameter, the DDL path is gated behind a non-null check, and the idempotency guard for the REMOVE TTL case uses stable dual anchoring.

The retention value is typed as int, validated positive at construction, and only ever interpolated as a bare integer literal into the SQL — there is no injection surface. The applyEventsRetention() call is sequenced after the events table creation, so it can never race with a missing table. The error-swallowing logic is tighter than in the first iteration (dual-anchored on code + keyword). No functional defects were identified.

No files require special attention.

Important Files Changed

Filename Overview
src/Usage/Adapter/ClickHouse.php Adds optional ?int $retention constructor parameter with positive-value guard; applyEventsRetention() issues MODIFY TTL or REMOVE TTL as an idempotent post-creation ALTER; error-swallowing for the no-TTL case is anchored on both Code: 36 and the word TTL to avoid catching unrelated BAD_ARGUMENTS errors; SQL interpolation of $this->retention is safe because PHP types it as int and the validation rejects non-positive values.
tests/Usage/Adapter/ClickHouseSchemaTest.php Adds two tests: testRetentionAppliesTtlToEventsTableOnly (integration, requires live ClickHouse) verifies the DDL contains TTL toDateTime(time) on the events table and no TTL on the daily table; testRetentionRejectsNonPositiveDays exercises constructor-time validation without a network connection via PHP 8 named argument syntax. Test isolation uses a unique namespace.

Reviews (2): Last reviewed commit: "fix(clickhouse): anchor REMOVE TTL guard..." | Re-trigger Greptile

Comment thread src/Usage/Adapter/ClickHouse.php
Comment thread tests/Usage/Adapter/ClickHouseSchemaTest.php
Match both the stable BAD_ARGUMENTS code (36) and a "TTL" mention instead
of the full English error phrase, so version-drift in the prose is tolerated
while unrelated bad-argument errors still surface.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@loks0n loks0n merged commit 93bf2e8 into utopia-php:main Jul 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants