feat(clickhouse): add optional retention TTL

[levivannoort]

What

Adds setRetention(?int $days) to the ClickHouse audit adapter. When set, setup() applies an idempotent TTL to the audit table so rows past the retention window are dropped by background merges:

$adapter->setRetention(30);
$audit->setup(); // ALTER TABLE ... MODIFY TTL toDateTime(time) + INTERVAL 30 DAY

Default null preserves current behaviour (no TTL).

Why in setup()

Retention is part of the table's schema, and setup() already owns everything else about it (columns, indexes, engine, partition key). Keeping TTL here means every caller that already runs setup() gets retention with no extra plumbing, and the table-naming logic isn't duplicated downstream.

How

• Applied as a separate ALTER … MODIFY TTL, not baked into CREATE TABLE, because CREATE TABLE IF NOT EXISTS won't touch an already-existing table. MODIFY TTL is a no-op when the expression is unchanged, so setup() stays re-runnable/idempotent.
• SETTINGS materialize_ttl_after_modify = 0 defers the purge to normal background merges instead of an immediate, I/O-heavy mutation.
• time is DateTime64(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

[greptile-apps]

Greptile Summary

This PR adds optional retention TTL support to the ClickHouse audit adapter via setRetention(?int $days) and applies it as a separate idempotent ALTER TABLE … MODIFY TTL inside setup(), with a corresponding REMOVE TTL path when retention is set back to null.

• setRetention validates that days are positive and stores the value; setup() applies or removes the TTL after the CREATE TABLE IF NOT EXISTS so existing tables are handled correctly.
• The REMOVE TTL else-branch runs unconditionally whenever $retention is null, causing an extra DB round-trip and a caught-and-swallowed exception on every setup() call for the common case of no retention configured.
• Four unit tests cover the getter/setter contract and the zero/negative-day guard clause.

Confidence Score: 5/5

The change is safe to merge; the new TTL logic is isolated to setup() and the setter/getter pair, with no impact on the read or write hot paths.

The core feature is correct and idempotent: MODIFY TTL is a no-op when the expression is unchanged, and REMOVE TTL properly strips an existing TTL. The two concerns raised are both non-blocking — the unconditional REMOVE TTL call adds a wasted round-trip for callers using the default null retention, and the error-message substring guard is fragile across ClickHouse versions. Neither causes wrong data or broken behaviour today.

The REMOVE TTL else-branch in src/Audit/Adapter/ClickHouse.php around line 837 is worth a second look for the unconditional overhead and the error-message matching strategy.

Important Files Changed

Filename	Overview
src/Audit/Adapter/ClickHouse.php	Adds setRetention(?int $days) / getRetention() and a post-CREATE ALTER TABLE TTL block in setup(); the REMOVE TTL else-branch correctly addresses the previous review thread but issues an unconditional extra round-trip for every caller using the default null retention
tests/Audit/Adapter/ClickHouseTest.php	Adds four unit tests covering the setter/getter contract, null-reset, and validation guards (zero and negative days)

_{Reviews (3): Last reviewed commit: "fix(clickhouse): tolerate REMOVE TTL on ..." | Re-trigger Greptile}

[greptile-apps]

No test coverage for the new retention feature

setRetention, getRetention, the validation path (negative/zero days → Exception), and the TTL branch inside setup() are not exercised by any test in ClickHouseTest. At minimum, a unit test for the setter/getter contract and the guard clause would protect against regressions without requiring a live ClickHouse instance.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!