feat(hogql): HogQLQuery node and data table support

[mariusandra]

Problem

HogQL is cool, but nobody can use it yet.

Extracted from #14042

Part of PostHog/meta#81

Stacked on top of #14185

Changes

Exposes a HogQL query editor, together with pretty printing and a table to show the results, under /query.

This HogQL editor is not exposed under its own node because it should likely always be created as a "new insight". Will wait for insights to natively be query nodes for that.

Security concerns

Before putting this live, I'd like to be 100% sure there's no way to leak data from other team. What was not resolved in #14185 will need to be discussed now.

• Is the team_id check secure as is? Can someone override it somehow? What must we look out for.
• Should put the backed query node behind a feature flag and enable just for our team out of paranoia?
• For joined tables, should this team_id guard be on the ON or in the WHERE clause?
• What about PREWHERE vs WHERE? Which should we actually use?

How did you test this code?

In the browser, plus added a test for the new API endpoint.

[mariusandra]

Other than a failing storybook build (sql formatter + webpack 4 = ❤️), this is ready for a review as well.

[thmsobrmlr]

LGTM

[thmsobrmlr]

I don't understand why these tests would change in this PR, but going to assume it's ok.

[mariusandra]

These two do look like unrelated changes. They're definitely referencing SQL that's not generated by HogQL. I remember hearing about some changes to /decide, so that's probably this plus a bad merge somewhere by someone?

[neilkakkar]

Yeah me 🙈 , old PR got merged after a snapshot change went in, which didn't update snapshots again

[thmsobrmlr]

Same here: I don't understand why these tests would change in this PR, but going to assume it's ok.

[neilkakkar]

Is the team_id check secure as is? Can someone override it somehow? What must we look out for.

It seems so. Potential thing to look out for: can I make conditions in such a way that the extra_where ends up as an OR condition? (Doesn't seem likely to me right now, as we explicitly always AND it with all the other expressions).

Should put the backed query node behind a feature flag and enable just for our team out of paranoia?

🤷‍♂️

For joined tables, should this team_id guard be on the ON or in the WHERE clause?

Definitely WHERE, faster and safer

Rows are joined if the whole complex condition is met. If the conditions are not met, still rows may be included in the result depending on the JOIN type. Note that if the same conditions are placed in a WHERE section and they are not met, then rows are always filtered out from the result.

What about PREWHERE vs WHERE? Which should we actually use?

It's fine to stick to WHERE, the only thing that would ever really go into PREWHERE is team_id check, but this is more of a perf optimisation.

[mariusandra]

Is the team_id check secure as is? Can someone override it somehow? What must we look out for.

It seems so. Potential thing to look out for: can I make conditions in such a way that the extra_where ends up as an OR condition? (Doesn't seem likely to me right now, as we explicitly always AND it with all the other expressions).

I can't imagine a case either. The only other case I can imagine is if somebody manually creates an alias with the name team_id for a random field to trick the where clause into accepting that, but since we do symbol resolution that's effectively impossible.

Out of paranoia I have anyway blocked xyx as team_id from working (ValueError: Alias 'team_id' is a reserved keyword.), but even removing the block, the query SELECT event, 1 as team_id from events translates to SELECT event, 1 AS team_id FROM events WHERE equals(events.team_id, 42) LIMIT 65535 thanks to symbol resolution. So I think we're good.

Should put the backed query node behind a feature flag and enable just for our team out of paranoia?

🤷‍♂️

I think if we implement some of the suggested query safeguards (max memory limit, timeouts, etc) discussed here, we're probably good

For joined tables, should this team_id guard be on the ON or in the WHERE clause?

Definitely WHERE, faster and safer

👍

Rows are joined if the whole complex condition is met. If the conditions are not met, still rows may be included in the result depending on the JOIN type. Note that if the same conditions are placed in a WHERE section and they are not met, then rows are always filtered out from the result.

What about PREWHERE vs WHERE? Which should we actually use?

It's fine to stick to WHERE, the only thing that would ever really go into PREWHERE is team_id check, but this is more of a perf optimisation.

Worth doing then, though I propose it should be outside the scope of this PR.

[mariusandra]

All right, to make everything simple and to give us the maximum amount of playroom, I put the backend in this PR behind a new flag hogql-queries. Locally (not is_cloud()) it's always enabled. On cloud, we check the flag before giving access to the HogQL backend.

The "frontend" is still as it is, since there's no real frontend, only an obscure /query debug URL where you'll find HogQL as the 21st link in a list of links. So I think that's fine for now. This flag can be reused when we build a real frontend, or get HogQL insights running.

I'll be happy to add all the query/byte/time/etc limits in a followup.