-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(hogql): support UNION ALL #14593
Conversation
posthog/hogql/printer.py
Outdated
@@ -137,6 +137,14 @@ def visit_select_query(self, node: ast.SelectQuery): | |||
if node.limit_with_ties: | |||
clauses.append("WITH TIES") | |||
|
|||
if len(node.union_all or []) > 0: | |||
# :TRICKY: union queries are not "children" the parent select, but "siblings". Adjust the stack for the visit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps UNION
should be a level above SELECT
? That's how ClickHouse does this (https://github.com/ClickHouse/ClickHouse/blob/6d48bb25ff2519b84306a7abb1c17ffe60034f20/src/Analyzer/UnionNode.h) – there's UnionNode
(which represents either UNION
, EXCEPT
, or INTERSECT
) with the constituent queries as children
. Then on the query tree level, they treat SELECT ...
and SELECT ... UNION SELECT ...
as interchangeable constructs (https://github.com/ClickHouse/ClickHouse/blob/6d48bb25ff2519b84306a7abb1c17ffe60034f20/src/Analyzer/QueryTreeBuilder.cpp#LL142C33-L142C33).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that makes sense. I've updated the code to do just that. Ready for another look.
Changes
Support
UNION ALL
syntax.How did you test this code?
Added a few tests