Skip to content

Commit

Permalink
fix(hogql): Fixed being able to use placeholder nodes in CTEs (#20273)
Browse files Browse the repository at this point in the history
* Fixed being able to use placeholder nodes in CTEs

* Added tests and fixed mypy
  • Loading branch information
Gilbert09 authored Feb 12, 2024
1 parent f663235 commit 0adc049
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 3 deletions.
10 changes: 8 additions & 2 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@ posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incomp
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "JoinConstraint | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "JoinExpr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "JoinExpr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "CTE") [assignment]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "CTE") [assignment]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "Expr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "Expr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "Expr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "CTE") [assignment]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "OrderExpr", variable has type "CTE") [assignment]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "Expr", variable has type "CTE") [assignment]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "Expr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "Expr | None"; expected "AST" [arg-type]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "WindowExpr", variable has type "CTE") [assignment]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "FieldAliasType", variable has type "BaseTableType | SelectUnionQueryType | SelectQueryType | SelectQueryAliasType") [assignment]
posthog/hogql/visitor.py:0: error: Incompatible types in assignment (expression has type "Type", variable has type "BaseTableType | SelectUnionQueryType | SelectQueryType | SelectQueryAliasType") [assignment]
posthog/hogql/visitor.py:0: error: Argument 1 to "visit" of "Visitor" has incompatible type "WindowFrameExpr | None"; expected "AST" [arg-type]
Expand Down Expand Up @@ -301,8 +307,6 @@ posthog/hogql/transforms/in_cohort.py:0: error: Item "None" of "JoinConstraint |
posthog/hogql/transforms/in_cohort.py:0: error: Item "Expr" of "Expr | Any" has no attribute "right" [union-attr]
posthog/hogql/transforms/in_cohort.py:0: error: List item 0 has incompatible type "SelectQueryType | None"; expected "SelectQueryType" [list-item]
posthog/hogql/transforms/in_cohort.py:0: error: List item 0 has incompatible type "SelectQueryType | None"; expected "SelectQueryType" [list-item]
posthog/api/organization.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/api/organization.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/hogql/printer.py:0: error: Argument 1 to "create_hogql_database" has incompatible type "int | None"; expected "int" [arg-type]
posthog/hogql/printer.py:0: error: List comprehension has incompatible type List[SelectQueryType | None]; expected List[SelectQueryType] [misc]
posthog/hogql/printer.py:0: error: Argument "stack" to "_Printer" has incompatible type "list[SelectQuery]"; expected "list[AST] | None" [arg-type]
Expand Down Expand Up @@ -333,6 +337,8 @@ posthog/hogql/printer.py:0: error: Non-overlapping equality check (left operand
posthog/hogql/printer.py:0: error: Argument 2 to "_get_materialized_column" of "_Printer" has incompatible type "str | int"; expected "str" [arg-type]
posthog/hogql/printer.py:0: error: Statement is unreachable [unreachable]
posthog/hogql/printer.py:0: error: Argument 1 to "_print_identifier" of "_Printer" has incompatible type "str | None"; expected "str" [arg-type]
posthog/api/organization.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/api/organization.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/hogql/hogql.py:0: error: Item "None" of "JoinExpr | None" has no attribute "alias" [union-attr]
posthog/api/team.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases [misc]
posthog/models/property/util.py:0: error: Argument 3 to "format_filter_query" has incompatible type "HogQLContext | None"; expected "HogQLContext" [arg-type]
Expand Down
3 changes: 3 additions & 0 deletions posthog/hogql/placeholders.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ def __init__(self):
super().__init__()
self.found: set[str] = set()

def visit_cte(self, node: ast.CTE):
super().visit(node.expr)

def visit_placeholder(self, node: ast.Placeholder):
self.found.add(node.field)

Expand Down
16 changes: 15 additions & 1 deletion posthog/hogql/test/test_placeholders.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import cast
from posthog.hogql import ast
from posthog.hogql.errors import HogQLException
from posthog.hogql.parser import parse_expr
from posthog.hogql.parser import parse_expr, parse_select
from posthog.hogql.placeholders import replace_placeholders, find_placeholders
from posthog.test.base import BaseTest

Expand Down Expand Up @@ -69,3 +70,16 @@ def test_assert_no_placeholders(self):
"Placeholders, such as {foo}, are not supported in this context",
str(context.exception),
)

def test_replace_placeholders_with_cte(self):
expr = cast(ast.SelectQuery, parse_select("with test as (select {foo}) select * from test"))

assert expr.ctes is not None and expr.ctes["test"] is not None
assert isinstance(expr.ctes["test"].expr, ast.SelectQuery)
assert isinstance(expr.ctes["test"].expr.select[0], ast.Placeholder)

expr2 = cast(ast.SelectQuery, replace_placeholders(expr, {"foo": ast.Constant(value=1)}))

assert expr2.ctes is not None and expr2.ctes["test"] is not None
assert isinstance(expr2.ctes["test"].expr, ast.SelectQuery)
assert isinstance(expr2.ctes["test"].expr.select[0], ast.Constant)
3 changes: 3 additions & 0 deletions posthog/hogql/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ def visit_join_expr(self, node: ast.JoinExpr):
def visit_select_query(self, node: ast.SelectQuery):
# :TRICKY: when adding new fields, also add them to visit_select_query of resolver.py
self.visit(node.select_from)
if node.ctes is not None:
for expr in list(node.ctes.values()):
self.visit(expr)
for expr in node.array_join_list or []:
self.visit(expr)
for expr in node.select or []:
Expand Down

0 comments on commit 0adc049

Please sign in to comment.