fix(hogql): dateAdd should be date_add

[EDsCODE]

Problem

• dateAdd doesn't work
• https://posthoghelp.zendesk.com/agent/tickets/14752

Changes

• date_add does

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

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

How did you test this code?

[github-actions]

Size Change: 0 B

Total Size: 1.06 MB

ℹ️ View Unchanged

Filename	Size
frontend/dist/toolbar.js	1.06 MB

_{compressed-size-action}

[mariusandra]

Questions.

1. Why? The PR doesn't say.
2. What will break?

[Gilbert09]

dateAdd is an alias if date_add - so I don't see why this wouldn't work. docs. I'd be keen to not change this if we can help it, it'll become one of the only funcs in snake case, and may break existing queries

[Gilbert09]

[EDsCODE]

Questions.

1. Why? The PR doesn't say.
2. What will break?

1. Hmm, so just sifted through clickhouse source and found more examples. When the bug was first reported, I basically tried all variations of dateAdd and nothing worked even though documentation suggests something like SELECT dateAdd('YEAR', 1, toDateTime('2022-01-01')) would work but this didn't work in hogQL or in clickhouse at all. But it looks like SELECT dateAdd(YEAR, 1, toDateTime('2022-01-01')) is the syntax that works on clickhouse but not on hogQL because 'YEAR' token without quotes isn't supported.

I changed it to date_add because SELECT date_add(toDateTime('2022-01-01'), INTERVAL 1 YEAR) which was the only formulation that seemed to work with hogQL. I didn't realize what I just found out this morning about dateAdd, it seemed like it was entirely broken in clickhouse.

So, I guess with that understanding, the better solution is to keep the existing signature but make sure dateAdd supports 3 arguments or 2 (as suggested in docs) and for 3 arguments, the first argument can be a quoteless time interval token. @mariusandra

2. From the support ticket it seems like nothing that isn't already? They tried just about all the formulations for dateAdd and nothing works

[mariusandra]

We definitely certainly absolutely do not want to remove functions from HogQL. This PR removes dateAdd. Did you check how many people use it in their insights, API queries and notebooks before proposing this? How many of them will have something that'll now break and ends up on the Product Analytics support queue?

The new function also does not look like any of the other functions we have. If you really want to write date_add in ClickHouse, change what dateAdd prints to, but there's no reason to change the signature in HogQL.

Will swapping INTERVAL 1 YEAR --> toIntervalYear(1) make a difference?

[EDsCODE]

Did you check how many people use it in their insights, API queries and notebooks before proposing this? How many of them will have something that'll now break and ends up on the Product Analytics support queue?

No, because the function entirely doesn't work so I was working with the assumption that it's 0 and/or no one reported it. Just checked and here's the data (please check my query logic in case I might be missing how hogql queries are stored)

1 on US (which noted that dateAdd doesn't work)

0 on EU

The new function also does not look like any of the other functions we have. If you really want to write date_add in ClickHouse, change what dateAdd prints to, but there's no reason to change the signature in HogQL.

Will swapping INTERVAL 1 YEAR --> toIntervalYear(1) make a difference?

Agreed the snake case doesn't make sense. I had just changed it to match clickhouse more closely.

We can keep dateAdd but can we change the function to accept only 2 args? That would work. Right now with 3, it expects an interval token as the first argument like YEAR and it doesn't work. INTERVAL 1 YEAR isn't the issue. That argument works but you can't pass that into the 3 argument version of dateAdd.

If we want to keep the signature exactly the same, I think we would need to support YEAR | MONTH | WEEK | DAY | MINUTE as its own AST node and valid in specific functions.

[mariusandra]

I'm more than happy to change the wrong number of arguments to the correct one 😄