Repository navigation
fix(query-cache): ignore removed modifiers when reading cached responses - #123931
sampennington wants to merge 1 commit into
Conversation
Every cached response stores the modifiers it ran with. HogQLQueryModifiers forbids extra fields, so removing a modifier from the schema made every entry written before the removal fail to parse and count as a cache miss. Drop modifier keys the schema does not know before parsing. Co-Authored-By: Claude Opus 5.5 <[email protected]> Generated-By: PostHog Desktop Task-Id: 876dcdf1-ca2f-410c-8aeb-e4649262f74b
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report
|
|
Closing in favor of #124050. The team chose to keep recalculating entries that no longer parse, and to show the cost of a field removal before merge instead of changing how old entries are read. |
Problem
When a modifier is removed from
HogQLQueryModifiers, customers lose their cached query results. Every cached response stores the modifiers it ran with, and the model usesextra="forbid". So each entry written before the removal fails to parse.handle_cache_and_async_logicthen counts it as a cache miss. Insights and dashboards calculate again, and stale-while-revalidate stops for these entries until they are written again.#122489 removed
useNewEventsClusterand caused this. Raised in this Slack thread.Changes
HogQLQueryModifiersdoes not know. This applies to all removed modifiers, not onlyuseNewEventsCluster. It follows the same filter thatcreate_default_modifiers_for_teamalready applies to stored team modifiers.#123812 fixes the same incident in a different way: it adds
useNewEventsClusterback as a deprecated field. This PR is smaller, needs no schema or type regeneration, and also prevents the problem for the next removed modifier. The two can land together.How did you test this code?
hogli test posthog/hogql_queries/test/test_query_runner.py -k "removed_from_the_schema or schema_change_triggers or test_cache_response"in the sandbox.query_runner.py.Test rationale:
test_a_modifier_removed_from_the_schema_keeps_the_cache_hitfails if a cached response with an unknown modifier key is treated as a miss again. The closest existing test,test_schema_change_triggers_recalculation, checks the opposite case: a response that really does not match the schema must still calculate again.Release status
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: PostHog Slack app (Claude Code), Claude Opus 5.5 (
claude-opus-5-5)Created with PostHog from a Slack thread
🤖 Generated with Claude Code