posthog

posthog copied to clipboard

Problem

https://github.com/PostHog/posthog/issues/20660

Changes

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

Automated tests

🔍 Existing Issues For Review

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

📄 File: posthog/hogql/printer.py

Function	Unhandled Issue
visit_join_expr	QueryError: Table function 'numbers' requires arguments posthog.tasks.tasks.process_que... Event Count: 1

📄 File: posthog/hogql/resolver.py (Click to Expand)

Function	Unhandled Issue
visit_join_expr	QueryError: Unknown table "e". posthog.tasks.task... Event Count: 8
visit_join_expr	QueryError: Unknown table "customer_value". posth... Event Count: 2
visit_join_expr	QueryError: Unknown table "stripe_invoice". posth... Event Count: 2
visit_join_expr	QueryError: Already have joined a table called "polls". Can't join another one with the same name. ... Event Count: 2

---

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

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

Unstale please

@thmsobrmlr PTAL!

About the failing tests - it seems like e2e tests don't recompile the c++ parser. These tests fail with the following error

2024-05-10T07:14:19.988440Z [error    ] JoinConstraint.__init__() missing 1 required keyword-only argument: 'constraint_type' [posthog.exceptions] host=localhost:8000 ip=127.0.0.1 path=/api/projects/1/query/ pid=141 request_id=20271ca5-215d-47fd-82af-c9df1a267eee team_id=1 tid=139837938559296 x_forwarded_for=
Traceback (most recent call last):
  File "/python-runtime/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/code/./posthog/api/query.py", line 92, in create
    raise e
  File "/code/./posthog/api/query.py", line 79, in create
    result = process_query_model(
  File "/code/./posthog/api/services/query.py", line 92, in process_query_model
    result = query_runner.run(execution_mode=execution_mode)
  File "/code/./posthog/hogql_queries/query_runner.py", line 377, in run
    fresh_response_dict = self.calculate().model_dump()
  File "/code/./posthog/hogql_queries/insights/retention_query_runner.py", line 309, in calculate
    query = self.to_query()
  File "/code/./posthog/hogql_queries/insights/retention_query_runner.py", line 245, in to_query
    "actor_query": self.actor_query(),
  File "/code/./posthog/hogql_queries/insights/retention_query_runner.py", line 207, in actor_query
    return parse_select(
  File "/code/./posthog/hogql/parser.py", line 100, in parse_select
    node = RULE_TO_PARSE_FUNCTION[backend]["select"](statement)
  File "/code/./posthog/hogql/parser.py", line 32, in <lambda>
    "select": lambda string: _parse_select_cpp(string),
TypeError: JoinConstraint.__init__() missing 1 required keyword-only argument: 'constraint_type'

But they finish successfully for me locally. Maybe there's a way to fix this?

@thmsobrmlr PTAL!

About the failing tests - it seems like e2e tests don't recompile the c++ parser. These tests fail with the following error

2024-05-10T07:14:19.988440Z [error    ] JoinConstraint.__init__() missing 1 required keyword-only argument: 'constraint_type' [posthog.exceptions] host=localhost:8000 ip=127.0.0.1 path=/api/projects/1/query/ pid=141 request_id=20271ca5-215d-47fd-82af-c9df1a267eee team_id=1 tid=139837938559296 x_forwarded_for=
Traceback (most recent call last):
  File "/python-runtime/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/code/./posthog/api/query.py", line 92, in create
    raise e
  File "/code/./posthog/api/query.py", line 79, in create
    result = process_query_model(
  File "/code/./posthog/api/services/query.py", line 92, in process_query_model
    result = query_runner.run(execution_mode=execution_mode)
  File "/code/./posthog/hogql_queries/query_runner.py", line 377, in run
    fresh_response_dict = self.calculate().model_dump()
  File "/code/./posthog/hogql_queries/insights/retention_query_runner.py", line 309, in calculate
    query = self.to_query()
  File "/code/./posthog/hogql_queries/insights/retention_query_runner.py", line 245, in to_query
    "actor_query": self.actor_query(),
  File "/code/./posthog/hogql_queries/insights/retention_query_runner.py", line 207, in actor_query
    return parse_select(
  File "/code/./posthog/hogql/parser.py", line 100, in parse_select
    node = RULE_TO_PARSE_FUNCTION[backend]["select"](statement)
  File "/code/./posthog/hogql/parser.py", line 32, in <lambda>
    "select": lambda string: _parse_select_cpp(string),
TypeError: JoinConstraint.__init__() missing 1 required keyword-only argument: 'constraint_type'

But they finish successfully for me locally. Maybe there's a way to fix this?

@Twixes Any hints on what's going on here?

Ah, that's because a new version of hogql_parser wasn't published to PyPI. This is normally easy and automated, requiring just a bump in hogql_parser/setup.py – but that only works on branches created in this repo itself, not on forks (due to PyPI authentication). :/

So one of us from PostHog needs to create a separate PR with just the C++ changes to ship that new version of the package (unfortunately it's not really feasible locally, because we need to build wheels for both Linux and macOS * both x86 and ARM).

Or it can be this whole branch basically, just with the hogql_parser version bumped, and then we'll be able to use the newly-released version in this PR.

Merged with https://github.com/PostHog/posthog/pull/22378. Thanks again for the great work @nikitaevg !