HogQL functions should ignore casing COALESCE vs coalesce

[thmsobrmlr]

Is your feature request related to a problem?

Currently HogQL functions need to be camelCased, where's ClickHouse doesn't really care.

Describe the solution you'd like

COALESCE should work the same as coalesce.

[nikitaevg]

Hey @thmsobrmlr

I want to start contributing to PostHog, I came across this task, it looks very good as a first issue. Is this still needed? Do you mind if I try it?

Regarding the approach, as per the ClickHouse documentation it does care about case for some functions. I drafted a PR, could you please take a look, does the approach look good to you? If so, I'll apply the new change to other functions that are case insensitive.

[thmsobrmlr]

Hey @thmsobrmlr

I want to start contributing to PostHog, I came across this task, it looks very good as a first issue. Is this still needed? Do you mind if I try it?

Regarding the approach, as per the ClickHouse documentation it does care about case for some functions. I drafted a PR, could you please take a look, does the approach look good to you? If so, I'll apply the new change to other functions that are case insensitive.

Hey @nikitaevg, first off sorry for the delay in response - it has been quite busy here. I was finally able to have a look at your PR and it looks very solid. After letting CI loose on it there are some lint warnings. Nothing else blocking from merging once these are fixed. You can ignore the other failing test, that is a flappy one and probably fixed in master.

[nikitaevg]

@thmsobrmlr No worries and thanks for the review

I fixed the lint warnings and made more functions case insensitive, PTAL!

Also, I think it makes sense to perform the same operation with the aggregations, to make things consistent, WDYT?

[thmsobrmlr]

@thmsobrmlr No worries and thanks for the review

I fixed the lint warnings and made more functions case insensitive, PTAL!

Also, I think it makes sense to perform the same operation with the aggregations, to make things consistent, WDYT?

Hey @nikitaevg, all looks good to me now and I've merged the PR already this morning. Doing the same thing with aggregations makes sense, yes.

Thanks for your contribution!

[thmsobrmlr]

Tracking the follow up here #21469, since I've been asked why some functions care about casing whereas others don't.