posthog

posthog copied to clipboard

Problem

If the query fails and returns None for results, we should still get back the HogQL query output and whatever else we got.

Sentry issue

ValidationError
1 validation error for HogQLQueryResponse
results
  Input should be a valid list [type=list_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/list_type

Changes

None or []

How did you test this code?

Didn't really, hoping it works.

🔍 Existing Issues For Review

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

📄 File: posthog/hogql/query.py

Function	Unhandled Issue
execute_hogql_query	CHQueryErrorUnknownIdentifier: DB::Exception: Missing columns: '--events__override.distinct_id' '--events__override.person_id'... ... Event Count: 2.3k
execute_hogql_query	CHQueryErrorNoCommonType: DB::Exception: There is no supertype for types String, Float64 because some of them are String/Fi... ... Event Count: 180
execute_hogql_query	CHQueryErrorTypeMismatch: DB::Exception: Cannot convert string 17.4.1 to type Float64: while executing 'FUNCTION equals(toF... ... Event Count: 152
execute_hogql_query	CHQueryErrorIllegalTypeOfArgument: DB::Exception: Illegal type String of argument for aggregate function avg. Stack trace: ... Event Count: 108
execute_hogql_query	ValidationError: 1 validation error for HogQLQueryResponse posthog.tasks.tasks.process_... Event Count: 86

---

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

This change concerns results not result 😅 🙈.

Alternatively I'll make results optional/nullable in the type, which is probably the right thing anyway.

Thinking this through, making results optional here will make results optional for all data nodes:

https://github.com/PostHog/posthog/blob/add62bedade42ccf4302f3e7c3ec058672c86640/frontend/src/queries/schema.ts#L899-L900

This change here is only for debug mode, where we want to return incomplete results if the query errors. It seems a bit excessive to add ?. to a lot of places in the frontend that deal with any kind of results just for this.

So I moved the [] line up a bit to make it more clear that it's an one off. Would this be fine?