chore(hogql): Add retention queries in HogQL #18831

webjunkie · 2023-11-22T20:36:05Z

Problem

We want to convert the retention queries to use HogQL as well.

Changes

make new retention query runner

How did you test this code?

test in frontend using feature flag
copied tests from old retention query and adapted them, made all pass

webjunkie · 2023-11-28T14:00:50Z

@mariusandra This is a bit rough from the code maybe, but mostly working. The tests fail at some poe stuff. Let's maybe discuss that tomorrow. Otherwise: Did I miss anything big?

mariusandra

Didn't get to the end yet, but sharing the comments I already have.

mariusandra · 2023-11-29T09:08:46Z

posthog/models/team/team.py

-    @cached_property
+    @property


Hmm, why this change?

It tripped up the tests. The test uses a team, asserts something, then sets the team timezone, and tries to assert once more. But due to the timezone property that the test uses being cached here on the instance, it didn't reflect the change. Not sure why it worked in the old tests.

But, I wouldn't recommend to put cached_property on "simple" properties like these anyways, since they are directly computed from available data and do not need to be queried for or such. It's often times more a source of confusion and stale data than helping performance. Hence I removed it. What do you think?

mariusandra · 2023-11-29T09:12:24Z

frontend/src/queries/schema.ts

@@ -498,8 +498,13 @@ export interface FunnelsQuery extends InsightsQueryBase {

 /** `RetentionFilterType` minus everything inherited from `FilterType` */
 export type RetentionFilter = Omit<RetentionFilterType, keyof FilterType>
+
+export interface RetentionQueryResponse extends QueryResponse {
+    results: any[]


Would be great to have this a bit more complete

posthog/hogql_queries/insights/retention_query_runner.py

# Conflicts: # frontend/src/queries/utils.ts

posthog-bot · 2023-12-01T13:25:30Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T12:50:36Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T12:59:20Z

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

chromium: 0 added, 3 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:07:54Z

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

chromium: 0 added, 3 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:16:17Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:24:50Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:33:21Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:41:57Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:50:28Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T13:59:40Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-12-06T14:08:58Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

mariusandra

I think this looks good! Great work! 👍

Still some tests / types to fix though, and then the persons modal stuff... but otherwise, let's get it in!

mariusandra · 2023-12-06T15:16:59Z

frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts

            }
+            result.retentionFilter = {} // Put this here to make TS happy


It seems to make TS unhappy...
I don't understand why we need it. Shouldn't filtersToQueryNode take care of stuff like that?

I have the query typed to require retentionFilter, hence added it, but since it's empty the result var didn't contain it and the equal test was failing. Anyway, I just used a cast to bypass issues now.

# Conflicts: # frontend/__snapshots__/scenes-app-sidepanels--side-panel-settings.png

webjunkie added 3 commits November 21, 2023 15:38

Set up retention query runnner

51bdfd0

Adjust date ranges

d7d7c3f

Fix date logic

53fb2cf

webjunkie changed the title ~~chore(hogql): Add retention queries as HogQL~~ chore(hogql): Add retention queries in HogQL Nov 22, 2023

webjunkie added 4 commits November 24, 2023 14:02

Use and adapt existing code

ebfb46c

Clean up use of properties and filters

a9de5cc

Streamline things by using HogQL AST

d035528

Add existing tests and adapt them

bffc6e2

mariusandra reviewed Nov 29, 2023

View reviewed changes

mariusandra requested a review from Gilbert09 November 29, 2023 09:39

Gilbert09 reviewed Nov 29, 2023

View reviewed changes

webjunkie and others added 12 commits November 29, 2023 14:06

Adjust query date range

c56b5fb

Adjust schema, remove alias

8c06ef9

Remove reliance on old filter

594ee7a

Clean up

2a4fcdc

Merge branch 'master' into chore/hogql-retention

b98d0e0

# Conflicts: # frontend/src/queries/utils.ts

Use more AST and clean up

fe4d1d8

Add feature flag for insights retention to frontend

19413c3

Clean up date filter

bece900

Make more tests work

4f08651

Make 100% of tests pass

e6b7005

Remove use of old mixin

65f24f4

Update UI snapshots for chromium (1)

67234a3

webjunkie added 5 commits December 4, 2023 09:29

Add more types

38af739

Remove people_url from result

f47a920

Fix lifecycle timezone test

7059ca8

Refactor self.event_query_type

22be1ce

Refactor

e0ccba4

Update UI snapshots for chromium (1)

4cf65bd

Update UI snapshots for chromium (1)

e331782

webjunkie and others added 2 commits December 6, 2023 13:15

Merge branch 'master' into chore/hogql-retention

02180eb

Update UI snapshots for chromium (1)

fa20f1d

Update UI snapshots for chromium (1)

120b0bc

Update UI snapshots for chromium (1)

c943cab

Update UI snapshots for chromium (1)

3bcffbc

Update UI snapshots for chromium (1)

95930d9

webjunkie and others added 2 commits December 6, 2023 13:56

Merge branch 'master' into chore/hogql-retention

226f3f2

Update UI snapshots for chromium (2)

5d3ea7b

github-actions bot added 2 commits December 6, 2023 14:07

Update UI snapshots for chromium (1)

9c5e75e

Update UI snapshots for chromium (2)

8ec4112

mariusandra approved these changes Dec 6, 2023

View reviewed changes

webjunkie added 2 commits December 6, 2023 17:20

Merge branch 'master' into chore/hogql-retention

55a8199

# Conflicts: # frontend/__snapshots__/scenes-app-sidepanels--side-panel-settings.png

Fix type issues

2cb2e70

webjunkie enabled auto-merge (squash) December 6, 2023 16:28

webjunkie merged commit 65e5ba7 into master Dec 6, 2023
78 checks passed

webjunkie deleted the chore/hogql-retention branch December 6, 2023 16:42

mariusandra mentioned this pull request Dec 7, 2023

HogQL Insights PostHog/meta#130

Open

48 tasks

webjunkie mentioned this pull request Dec 11, 2023

feat(hogql): Add retention appearance to persons query runner #19211

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(hogql): Add retention queries in HogQL #18831

chore(hogql): Add retention queries in HogQL #18831

webjunkie commented Nov 22, 2023 •

edited

Loading

webjunkie commented Nov 28, 2023

mariusandra left a comment

mariusandra Nov 29, 2023

webjunkie Nov 29, 2023

mariusandra Nov 29, 2023

posthog-bot commented Dec 1, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

posthog-bot commented Dec 6, 2023

mariusandra left a comment

mariusandra Dec 6, 2023

webjunkie Dec 6, 2023

		}
		result.retentionFilter = {} // Put this here to make TS happy

chore(hogql): Add retention queries in HogQL #18831

chore(hogql): Add retention queries in HogQL #18831

Conversation

webjunkie commented Nov 22, 2023 • edited Loading

Problem

Changes

How did you test this code?

webjunkie commented Nov 28, 2023

mariusandra left a comment

Choose a reason for hiding this comment

mariusandra Nov 29, 2023

Choose a reason for hiding this comment

webjunkie Nov 29, 2023

Choose a reason for hiding this comment

mariusandra Nov 29, 2023

Choose a reason for hiding this comment

posthog-bot commented Dec 1, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

posthog-bot commented Dec 6, 2023

📸 UI snapshots have been updated

mariusandra left a comment

Choose a reason for hiding this comment

mariusandra Dec 6, 2023

Choose a reason for hiding this comment

webjunkie Dec 6, 2023

Choose a reason for hiding this comment

webjunkie commented Nov 22, 2023 •

edited

Loading