fix(clickhouse): render mutation params without connection context by bciaraldi · Pull Request #111086 · PostHog/posthog

bciaraldi · 2026-10-02T19:00:09Z

Problem

Data deletion requests that carry a time range (event_removal in immediate mode) fail before any mutation is submitted, with 'NoneType' object has no attribute 'get_timezone', so the deletion never runs.

MutationRunner.find_existing_mutations renders each command with client.substitute_params(..., client.connection.context). For tz-aware datetime parameters, clickhouse-driver reads context.server_info.get_timezone(), but server_info stays None until the client has executed a query. Clients pulled fresh from the pool hit this on their first call.

Changes

Deletion requests with tz-aware time-range parameters now enqueue their mutations instead of failing.
Mechanical: find_existing_mutations connects a not-yet-connected client with SELECT 1, then renders with the client's own context, matching how client.execute renders the submitted mutation in the server timezone.
SELECT 1 is used instead of force_connect(), which leaves the connection marked mid-query and makes the next execute raise PartiallyConsumedQueryError.
The $__sql$ heredoc protection is unchanged: rendered commands are still bound as ordinary parameters.
Connection and credentials are unchanged: the same pooled client connects slightly earlier, using the password stamped at pull. Already-connected clients skip the extra query.

How did you test this code?

pytest posthog/clickhouse/test/test_cluster.py -k "tz_aware or delimiter_shaped" run locally.
Not checked: a live retry of a failed deletion request; that happens after deploy.

Test rationale: test_find_existing_mutations_renders_tz_aware_datetimes_before_connecting covers an unconnected client with a tz-aware datetime against a non-UTC server, asserting the client connects first and the lookup renders in the server timezone like submission does. The closest existing test, test_find_existing_mutations_handles_delimiter_shaped_parameter_value, uses no datetimes, so it could not catch either failure. It runs against real ClickHouse and still passes, which confirms the connect path and the injection fix.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

No feature flag controls this change
This change is behind a feature flag and is not available to users
This change makes a previously flagged feature available to everyone

Automatic notifications

Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, claude-opus-5-5

Root cause traced from Dagster run logs and ClickHouse system.mutations / query log, then confirmed in the clickhouse-driver source.
First revision rendered with a fixed UTC context; review pointed out it could diverge from server-side rendering on non-UTC deployments, so the lookup now connects first and uses the server's context.
No repo skills invoked. CodeRabbit local pass skipped (not run in this session); CodeRabbit PR review finding addressed as above.
Duplicate-PR search not run from the agent session (GitHub search unavailable there); run by the author before opening.
Work originated from an internal support ticket; the test data in this PR is invented.

🤖 Generated with Claude Code

…ion-tz-param-render

trunk-io · 2026-10-02T19:00:13Z

😎 Merged successfully - details.

parameterai · 2026-10-02T19:00:17Z

Risk: No findings

The change makes find_existing_mutations connect a fresh pooled client with SELECT 1 before rendering mutation commands with the client's connection context, so tz-aware datetime parameters render in the server timezone and deletion requests no longer crash with NoneType ... get_timezone. The rendered command texts remain bound as ordinary %(__command_N)s parameters and the added query is a literal, so no injection or authorization surface changes.

_{Sentinel reviewed 77ad25e · Review settings}

greptile-apps · 2026-10-02T19:03:21Z

[Medium risk] Changes how mutation SQL parameters are escaped in the database layer.

The PR appears safe to merge.

_{Reviews (1) · Last reviewed commit: "Merge branch 'master' of github.com:Post..."}

coderabbitai · 2026-10-02T19:12:59Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

🧰 Additional context used

📚 Code guidelines (6)

.agents/security.md — configured
.agents/skills/sending-notifications/SKILL.md — configured
.agents/skills/writing-tests/SKILL.md — configured
docs/internal/person-data-access.md — configured
.claude/commands/conventions.md — configured
.agents/skills/writing-code-comments/SKILL.md — configured

📝 Walkthrough

Walkthrough

When client.connection.context.server_info is absent, find_existing_mutations executes SELECT 1 before rendering mutation commands with the connection context. A regression test checks the UTC timestamp conversion for a server in America/New_York.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 77ad2

Time-range deletion lookups with timezone-aware datetimes should now work on a fresh ClickHouse client. No merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 77ad2

The change restores timezone-aware deletion requests through the existing database client and parameter controls. No expanded database authority or new security issue was demonstrated. Recovery after an interrupted initialization query remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The effective change is within existing ClickHouse mutation workflows, including data deletion across their selected hosts or shards. It restores previously failing requests rather than introducing a new caller path or widening the selected database, table, or tenant predicate.

Trust Boundaries and Controls

inferred — SELECT 1 contains no attacker-controlled input and executes through the same client used for lookup and submission. The reviewed change does not select another identity or host. Existing credential stamping occurs at pool checkout, and command values continue through the existing parameter controls.

Resilience and Maintainability Implications

observed — Cluster dispatch scopes client use with a pool context manager and re-raises execution failures. Its retry policy wraps the callback within that checkout, so a retry can reuse the client. Safe recovery from an interrupted initialization query depends on native driver and pool behavior that this review could not establish.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is complete and stand-alone. It explains the problem, user-visible impact, implementation, testing, test rationale, release status, documentation status, and agent context. It also rec…

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 4f9d5e6e-a4cb-4173-b4c2-962b92389689

📥 Commits

Reviewing files that changed from the base of the PR and between ff2ef48 and f8bfb4c.

📒 Files selected for processing (2)

posthog/clickhouse/cluster.py
posthog/clickhouse/test/test_cluster.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

hosthog · 2026-10-02T21:13:40Z

HostHog preview — `posthog-desktop-web`

The previews for this PR have been torn down and no longer serve.

github-actions · 2026-10-02T22:08:04Z

🤖 CI report

⚠️

Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Playwright — all passed

All tests passed.

View test results →

…posthog into bc/fix-mutation-tz-param-render

coderabbitai

🧹 Nitpick comments (1)

posthog/clickhouse/test/test_cluster.py (1)

430-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

State the invariant, not the change history.

This docstring describes how the previous implementation failed. Replace that account with the test’s required behavior: a fresh client renders timezone-aware parameters in the server timezone. As per coding guidelines, “Never record how the code got here.”

Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 16788e6b-0492-4f23-aedd-177c0d8b10d9

📥 Commits

Reviewing files that changed from the base of the PR and between faf182f and 77ad25e.

📒 Files selected for processing (2)

posthog/clickhouse/cluster.py
posthog/clickhouse/test/test_cluster.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

trunk-io · 2026-10-02T22:25:46Z

_{View Full Report ↗︎ ⋅ Docs}

deployment-status-posthog · 2026-10-02T23:23:42Z

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-10-02 23:23 UTC	Run
prod-us	✅ Deployed	2026-10-02 23:32 UTC	Run
prod-eu	✅ Deployed	2026-10-02 23:34 UTC	Run

bciaraldi added 2 commits October 2, 2026 11:54

fix(clickhouse): render mutation params without connection context

a5258bd

Merge branch 'master' of github.com:PostHog/posthog into bc/fix-mutat…

f8bfb4c

…ion-tz-param-render

bciaraldi requested a review from a team October 2, 2026 19:03

tpgilmore approved these changes Oct 2, 2026

View reviewed changes

coderabbitai Bot reviewed Oct 2, 2026

View reviewed changes

Comment thread posthog/clickhouse/cluster.py Outdated

bciaraldi added 3 commits October 2, 2026 12:20

fix(clickhouse): render mutation lookup in the server timezone

68718ba

Merge branch 'master' into bc/fix-mutation-tz-param-render

a953d54

Merge branch 'master' into bc/fix-mutation-tz-param-render

21d585d

Merge branch 'master' into bc/fix-mutation-tz-param-render

faf182f

Merge branch 'bc/fix-mutation-tz-param-render' of github.com:PostHog/…

77ad25e

…posthog into bc/fix-mutation-tz-param-render

coderabbitai Bot reviewed Oct 2, 2026

View reviewed changes

trunk-io Bot merged commit 17bed35 into master Oct 2, 2026
286 checks passed

trunk-io Bot deleted the bc/fix-mutation-tz-param-render branch October 2, 2026 22:56

Navigation Menu

fix(clickhouse): render mutation params without connection context - #111086

fix(clickhouse): render mutation params without connection context#111086
trunk-io[bot] merged 7 commits into
masterfrom
bc/fix-mutation-tz-param-render

bciaraldi commented Oct 2, 2026 •

edited

Loading

Uh oh!

trunk-io Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

parameterai Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading

Reviews paused

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

hosthog Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

trunk-io Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

deployment-status-posthog Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bciaraldi commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

How did you test this code?

Release status

Automatic notifications

Docs update

🤖 Agent context

Uh oh!

trunk-io Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parameterai Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Security Architecture Review

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hosthog Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

HostHog preview — posthog-desktop-web

Uh oh!

github-actions Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 CI report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

trunk-io Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

deployment-status-posthog Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploy status

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bciaraldi commented Oct 2, 2026 •

edited

Loading

trunk-io Bot commented Oct 2, 2026 •

edited

Loading

parameterai Bot commented Oct 2, 2026 •

edited

Loading

greptile-apps Bot commented Oct 2, 2026 •

edited

Loading

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading

hosthog Bot commented Oct 2, 2026 •

edited

Loading

HostHog preview — `posthog-desktop-web`

github-actions Bot commented Oct 2, 2026 •

edited

Loading

trunk-io Bot commented Oct 2, 2026 •

edited

Loading

deployment-status-posthog Bot commented Oct 2, 2026 •

edited

Loading