posthog

posthog copied to clipboard

Problem

The HogQL version of the trends actors query has several problems. See https://github.com/PostHog/posthog/pull/22103.

Changes

This PR re-implements the actors query in a separate TrendsActorsQueryBuilder, thereby fixing a number of issues.

Follow ups

The following issues are out-or-scope for this PR (to get the other fixes in), and will be addressed in a follow up.

• [ ] test_trends_all_cohort_breakdown_persons - fails, as 'all' cohort can't be resolved
• [ ] test_trends_multi_cohort_breakdown_persons - fails, as cohort breakdown value is seemingly ignored
• [ ] check that the frontend passes in the correct time zone info or better omits it
• [ ] check that the breakdown subquery date range is correct also for compare_value and wau/mau options
• [ ] test_trends_breakdown_others_persons - fails, as other returns all breakdowns, even those that should be display with the breakdown_limit
• [ ] test_trends_math_property_sum_persons - fails, as event_count isn't populated properly
• [ ] test_trends_math_count_per_actor_persons - fails, as event_count isn't populated properly
• [ ] event_count for unique_group isn't correct
• [ ] add test_trends_math_sum_person
• [ ] implement includeRecordings=False

How did you test this code?

Added unit tests and clicked around

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: posthog/hogql_queries/insights/trends/trends_query_builder.py

Function	Unhandled Issue
_get_events_subquery	ValueError: could not convert string to float: '[2876.0,3177.01]' posthog.tasks.tasks.p... Event Count: 1
_inner_select_query	UnboundLocalError: local variable 'span' referenced before assignment posthog.tasks.tas... Event Count: 1

---

_{Did you find this useful? React with a 👍 or 👎}