chore(cohorts): drop the superseded profile-keyed summary MVs

[niajkitir]

Follow-up to #458, as discussed.

Why now

#458 replaced profile_event_summary_mv and profile_event_property_summary_mv with pairs keyed for the queries that actually read them (migration 20), filled those with history (migration 21), and pointed cohort.service at the new tables.

The old pair was deliberately left in place and kept receiving inserts, so the change stayed revertible by pointer while the new tables were verified. Nothing reads them now, so all they do is fire two extra MV triggers on every event insert for no consumer. This drops them.

What changes

23-drop-old-cohort-summary-mvs.ts drops the two views.

delete.service stops naming them. That part isn't cosmetic: ALTER TABLE ... DELETE against a dropped table is UNKNOWN_TABLE, so leaving those entries would break project deletion. Confirmed against the post-drop schema:

Code: 60. DB::Exception: Could not find table: profile_event_summary_mv. (UNKNOWN_TABLE)

Their TABLE_NAMES entries stay, with a comment saying why: migrations 13, 14 and 15 still reference them and have to keep compiling. Happy to inline the literals in those three and remove the constants instead, if you'd rather not carry them.

Clusters

A clustered install has two objects per MV: <name> is the Distributed table and <name>_replicated is the view itself. The Distributed table is dropped first so nothing can route a read at a view that's mid-drop. Dropping the view takes its implicit .inner_id.<uuid> storage with it, so there's no third name to clean up (verified: inner table count goes 8 → 6 on both nodes).

Testing

Standalone ClickHouse 26.1.3 and a keeper-backed 2-shard cluster on 25.3, each built from an empty database through migration 21 before dropping.

	standalone	2-shard cluster
Both MVs gone	yes	yes, on both nodes
Inner storage tables released	8 → 6	8 → 6 on both nodes
Cohort reads still resolve	50 profiles	50 from either node
Ingestion after the drop	continues	continues
Second run	no-op	no-op
Every statement delete.service emits	11/11 accepted	11/11 accepted

One-way

No down(). The aggregated history goes with the tables, and rerunning migrations 13 and 14 would only bring back empty structure, so a down() here would be a rollback in name only. The header notes where the definitions live if anyone ever needs them back.

Unrelated, but worth flagging

main currently has duplicate migration numbers: 20-cohort-summary-mv-sort-key alongside 20-invite-project-access-levels, and 21-backfill-cohort-summary-mvs alongside 21-wind-down-onboarding-pointer. migrate.ts sorts on the numeric prefix, so ties fall back to readdirSync order, which is filesystem-dependent. Harmless for these pairs since they're independent, but it's a latent ordering hazard. I numbered this one 23 to stay clear of 22-add-events-inserted-at.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Maintenance
  • Removed obsolete cohort summary data views from ClickHouse installations.
  • Updated data cleanup behavior to stop targeting retired summary views.
  • Preserved compatibility for existing migrations that reference legacy views.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b0430a8-902c-41b3-b617-8e57993cf8e8

📥 Commits

Reviewing files that changed from the base of the PR and between 663959d and be377dd.

📒 Files selected for processing (3)

• packages/db/code-migrations/23-drop-old-cohort-summary-mvs.ts
• packages/db/src/clickhouse/client.ts
• packages/db/src/services/delete.service.ts

💤 Files with no reviewable changes (1)

• packages/db/src/services/delete.service.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The PR adds migration 23 to drop two superseded ClickHouse materialized views. Clustered installations drop Distributed and replicated objects. Non-clustered installations drop the tables only. Project deletion no longer targets these views, while legacy migration references remain documented.

Changes

Superseded cohort summary views

Layer / File(s)	Summary
Drop superseded materialized views packages/db/code-migrations/23-drop-old-cohort-summary-mvs.ts	Migration 23 conditionally drops the obsolete materialized views for clustered and non-clustered ClickHouse installations.
Remove obsolete deletion references packages/db/src/services/delete.service.ts, packages/db/src/clickhouse/client.ts	Project deletion omits the dropped views. TABLE_NAMES retains the legacy name for earlier migrations and documents its replacement.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to be377

This PR removes two legacy cohort-summary views and stops project deletion from targeting them. Merge readiness is currently moderate because a standalone/cluster configuration mismatch could leave replicated views and stored data outside the deletion lifecycle, while dry-run execution can still perform irreversible drops.

Sequence Diagram(s)

sequenceDiagram
  participant Migration as migration 23
  participant ClusterConfig as getIsCluster
  participant ClickHouseRunner as runClickhouseMigrationCommands
  Migration->>ClusterConfig: read cluster mode
  Migration->>ClickHouseRunner: execute cluster-specific drop commands

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: dropping the superseded profile-keyed cohort summary materialized views.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}