feat(clickhouse): add optional retention TTL on the snapshot table#20
feat(clickhouse): add optional retention TTL on the snapshot table#20levivannoort wants to merge 3 commits into
Conversation
Add setRetention(?int $days) to the ClickHouse adapter. When set, setup() applies an idempotent `ALTER TABLE ... MODIFY TTL toDateTime(time) + INTERVAL <days> DAY` to the snapshot (ReplacingMergeTree) table so latest-state rows older than the window are dropped by background merges. - Only the snapshot table gets a TTL; the aggregated (SummingMergeTree) table is left untouched since it backs long-term usage/billing history. - Applied as a separate ALTER (not in CREATE TABLE) so it also covers already-existing tables; MODIFY TTL is a no-op when unchanged, keeping setup() re-runnable. - materialize_ttl_after_modify = 0 defers the purge to background merges instead of an immediate mutation. - Default null preserves existing behaviour (no TTL). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds an optional retention TTL to the ClickHouse snapshot table (
Confidence Score: 5/5Safe to merge; the TTL logic is well-scoped and idempotent, and the two previously identified issues have been corrected. The change is narrow (one file, two new public methods, one DDL block in setup()), the snapshot-only scope is clearly enforced, and setup() remains re-runnable in all three states: never had TTL, has TTL, had TTL removed. The only open point is that the catch block for REMOVE TTL matches an English error substring rather than a stable error code, but this does not affect current correctness. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(clickhouse): tolerate REMOVE TTL on ..." | Re-trigger Greptile |
…n private Addresses review feedback on the retention TTL: - setup() now issues ALTER TABLE ... REMOVE TTL on the null path so disabling retention actually strips a previously applied TTL, matching the documented "pass null to disable" contract. REMOVE TTL is a no-op on a table without a TTL, so setup() stays idempotent. - $retention is now private, matching the other sensitive fields and preventing subclasses from bypassing the positive-day guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
REMOVE TTL is not a no-op — ClickHouse raises error 36 (BAD_ARGUMENTS, "Table doesn't have any table TTL expression, cannot remove") when the snapshot table has no TTL. That broke setup() on every fresh table when retention is disabled. Swallow that specific error to keep setup() idempotent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Adds
setRetention(?int $days)to the ClickHouse usage adapter. When set,setup()applies an idempotent TTL to the snapshot (ReplacingMergeTree) table so latest-state rows past the retention window are dropped by background merges:Default
nullpreserves current behaviour (no TTL).Scope: snapshot only
Only the snapshot table gets a TTL. The aggregated
{ns}_usage(SummingMergeTree) table is intentionally left untouched — it backs long-term usage/billing history, so dropping rows there would delete billing data.Why in setup()
Retention is part of the table's schema, and
setup()already owns everything else about it. Keeping TTL here means callers that already runsetup()get retention with no extra plumbing, and no downstream code has to re-derive table names.How
ALTER … MODIFY TTL, not baked intoCREATE TABLE, sinceCREATE TABLE IF NOT EXISTSwon't touch an existing table.MODIFY TTLis a no-op when unchanged, sosetup()stays idempotent/re-runnable.SETTINGS materialize_ttl_after_modify = 0defers the purge to normal background merges instead of an immediate mutation.timeisDateTime64(3);toDateTime(time)narrows it for the TTL expression.setRetention()rejects non-positive day counts.Pint (
composer lint) and PHPStan level max (composer check) pass on the changed file.🤖 Generated with Claude Code