feat(hogql): refactor macros and asterisks into "resolver"

[mariusandra]

Still draft

Problem

Expanding macros and * fields in a separate step from the resolver was causing maintainability issues. We had 3 stages in parallel: expand_macros, resolve_types, expand_asterisks. For a good implementation, we'd need to call all 3 in parallel when adding something new in any of these steps --> new column added in expand_asterisks --> run all 3 again on the new node.

Changes

• Now that all three visitors (expand_macros, resolve_types, expand_asterisks) use CloningVisitor, they're easy to merge.
• Merges all expansion and resolving steps into resolver.py.
• Move transforms/asterisk.py and transforms/macros.py into resolver.py, adapt as needed
• Paves way for feat(hogql): custom SQL columns #15180

How did you test this code?

• All old tests work.
• Added some new ones.

[mariusandra]

This will be is already fixed in #15180

[Twixes]

Makes sense and looks good, there's just one thing I'm still not sure about: why we don't just use the standard "common table expression" for this? As this already is familiar in SQL world – definitely more than Lisp macros

[mariusandra]

That is a good question. We definitely could swap out the logic from "extract macros from WITH" to "embrace the WITH CTE-s". I initially embraced the macro approach since there were some ClickHouse limitations (e.g. no recursion) that I wanted to overcome.

I'm willing to give this another look, and have added "Do we want to keep extracting macros from the WITH clause, or embrace CTE-s?" as a checkbox to tick. I'd leave that refactor for another PR though :).

[Twixes]

Makes sense, let's get this in

[mariusandra]

Thanks! Very much related, when working on this - #15244 - any additional duplication of properties.foo such as if(properties.foo, properties.foo, elseMoo) resulted in a slowdown of the query. I didn't try it out, but could aliasing the repeating part into a WITH expression work as some kind of optimisation?