fix(cohorts): wrap date operator to fix DateTime property comparisons#53818
Draft
matheus-vb wants to merge 1 commit intomasterfrom
Draft
fix(cohorts): wrap date operator to fix DateTime property comparisons#53818matheus-vb wants to merge 1 commit intomasterfrom
matheus-vb wants to merge 1 commit intomasterfrom
Conversation
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Cohort calculation fails when filtering on a custom DateTime person property using
is_date_before,is_date_after, oris_date_exact. The cohort gets stuck showing "We're queuing the calculation. It should be ready in a few minutes." indefinitely. The underlying ClickHouse error isCannot convert string '2026-03-19T14:00:00Z' to type DateTime64(6, 'UTC').The root cause is an asymmetric type cast in the HogQL property filter code. The PropertySwapper wraps the left side (the property value) in
toDateTime()which producesDateTime64(6, 'UTC'), but the right side (the comparison value) is left as a raw string constant. ClickHouse cannot implicitly convert ISO 8601 strings withT/ZtoDateTime64.Changes
Split
IS_DATE_BEFORE,IS_DATE_AFTER, andIS_DATE_EXACTfrom their generic counterparts (LT,GT,EXACT) in_expr_to_compare_opand wrap the comparison value intoDateTime()for the date operators. This ensures the printer generatesparseDateTime64BestEffort('...', 6, 'UTC')on the right side, matching the left side's type. Generic comparison operators are unchanged since they're also used for numeric comparisons.How did you test this code?
Added unit tests in
posthog/hogql/test/test_property.pycovering all three date operators, verifying the RHS is wrapped intoDateTime(). Also added regression guards confirming genericlt/gtoperators remain unchanged. Ran the full HogQL property test suite (99 tests passed). Verified the new test fails on the unfixed code and passes with the fix.Publish to changelog?
No