feat(data-warehouse): Added the ability to use dashboard filters in B…

…I queries (#24033) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Michael Matloka <[email protected]>
PostHog · Jul 31, 2024 · 59a1523 · 59a1523
1 parent bef13f1
commit 59a1523
Show file tree

Hide file tree

Showing 20 changed files with 106 additions and 35 deletions.
diff --git a/hogql_parser/HogQLParser.cpp b/hogql_parser/HogQLParser.cpp
@@ -581,7 +581,7 @@ void hogqlparserParserInitialize() {
   	1265,5,105,0,0,1262,1265,3,152,76,0,1263,1265,3,154,77,0,1264,1261,1,
   	0,0,0,1264,1262,1,0,0,0,1264,1263,1,0,0,0,1265,161,1,0,0,0,1266,1267,
   	3,166,83,0,1267,1268,5,122,0,0,1268,1269,3,148,74,0,1269,163,1,0,0,0,
-  	1270,1271,5,128,0,0,1271,1272,3,160,80,0,1272,1273,5,147,0,0,1273,165,
+  	1270,1271,5,128,0,0,1271,1272,3,134,67,0,1272,1273,5,147,0,0,1273,165,
   	1,0,0,0,1274,1277,5,110,0,0,1275,1277,3,168,84,0,1276,1274,1,0,0,0,1276,
   	1275,1,0,0,0,1277,167,1,0,0,0,1278,1282,5,142,0,0,1279,1281,3,170,85,
   	0,1280,1279,1,0,0,0,1281,1284,1,0,0,0,1282,1280,1,0,0,0,1282,1283,1,0,
@@ -11733,8 +11733,8 @@ tree::TerminalNode* HogQLParser::PlaceholderContext::LBRACE() {
   return getToken(HogQLParser::LBRACE, 0);
 }
 
-HogQLParser::IdentifierContext* HogQLParser::PlaceholderContext::identifier() {
-  return getRuleContext<HogQLParser::IdentifierContext>(0);
+HogQLParser::NestedIdentifierContext* HogQLParser::PlaceholderContext::nestedIdentifier() {
+  return getRuleContext<HogQLParser::NestedIdentifierContext>(0);
 }
 
 tree::TerminalNode* HogQLParser::PlaceholderContext::RBRACE() {
@@ -11770,7 +11770,7 @@ HogQLParser::PlaceholderContext* HogQLParser::placeholder() {
     setState(1270);
     match(HogQLParser::LBRACE);
     setState(1271);
-    identifier();
+    nestedIdentifier();
     setState(1272);
     match(HogQLParser::RBRACE);
 

diff --git a/hogql_parser/HogQLParser.h b/hogql_parser/HogQLParser.h
@@ -2411,7 +2411,7 @@ class  HogQLParser : public antlr4::Parser {
     PlaceholderContext(antlr4::ParserRuleContext *parent, size_t invokingState);
     virtual size_t getRuleIndex() const override;
     antlr4::tree::TerminalNode *LBRACE();
-    IdentifierContext *identifier();
+    NestedIdentifierContext *nestedIdentifier();
     antlr4::tree::TerminalNode *RBRACE();
 
 

diff --git a/hogql_parser/HogQLParser.interp b/hogql_parser/HogQLParser.interp
diff --git a/hogql_parser/parser.cpp b/hogql_parser/parser.cpp
@@ -2554,8 +2554,11 @@ class HogQLParseTreeConverter : public HogQLParserBaseVisitor {
   }
 
   VISIT(Placeholder) {
-    string name = visitAsString(ctx->identifier());
-    RETURN_NEW_AST_NODE("Placeholder", "{s:s#}", "field", name.data(), name.size());
+    auto nested_identifier_ctx = ctx->nestedIdentifier();
+    vector<string> nested =
+        nested_identifier_ctx ? any_cast<vector<string>>(visit(nested_identifier_ctx)) : vector<string>();
+
+    RETURN_NEW_AST_NODE("Placeholder", "{s:N}", "chain", X_PyList_FromStrings(nested));
   }
 
   VISIT_UNSUPPORTED(EnumValue)

diff --git a/hogql_parser/setup.py b/hogql_parser/setup.py
@@ -32,7 +32,7 @@
 
 setup(
     name="hogql_parser",
-    version="1.0.32",
+    version="1.0.35",
     url="https://github.com/PostHog/posthog/tree/master/hogql_parser",
     author="PostHog Inc.",
     author_email="[email protected]",

diff --git a/posthog/hogql/ast.py b/posthog/hogql/ast.py
@@ -698,7 +698,11 @@ class Field(Expr):
 
 @dataclass(kw_only=True)
 class Placeholder(Expr):
-    field: str
+    chain: list[str | int]
+
+    @property
+    def field(self):
+        return ".".join(str(chain) for chain in self.chain)
 
 
 @dataclass(kw_only=True)

diff --git a/posthog/hogql/filters.py b/posthog/hogql/filters.py
@@ -1,5 +1,6 @@
 from typing import Optional, TypeVar
 
+import dataclasses
 from dateutil.parser import isoparse
 
 from posthog.hogql import ast
@@ -14,6 +15,12 @@
 T = TypeVar("T", bound=ast.Expr)
 
 
+@dataclasses.dataclass
+class CompareOperationWrapper:
+    compare_operation: ast.CompareOperation
+    skip: bool = False
+
+
 def replace_filters(node: T, filters: Optional[HogQLFilters], team: Team) -> T:
     return ReplaceFilters(filters, team).visit(node)
 
@@ -24,15 +31,28 @@ def __init__(self, filters: Optional[HogQLFilters], team: Team = None):
         self.filters = filters
         self.team = team
         self.selects: list[ast.SelectQuery] = []
+        self.compare_operations: list[CompareOperationWrapper] = []
 
     def visit_select_query(self, node):
         self.selects.append(node)
         node = super().visit_select_query(node)
         self.selects.pop()
         return node
 
+    def visit_compare_operation(self, node):
+        self.compare_operations.append(CompareOperationWrapper(compare_operation=node, skip=False))
+        node = super().visit_compare_operation(node)
+        compare_wrapper = self.compare_operations.pop()
+        if compare_wrapper.skip:
+            return ast.CompareOperation(
+                left=ast.Constant(value=True),
+                op=ast.CompareOperationOp.Eq,
+                right=ast.Constant(value=True),
+            )
+        return node
+
     def visit_placeholder(self, node):
-        if node.field == "filters":
+        if node.chain == ["filters"]:
             if self.filters is None:
                 return ast.Constant(value=True)
 
@@ -111,4 +131,40 @@ def visit_placeholder(self, node):
             if len(exprs) == 1:
                 return exprs[0]
             return ast.And(exprs=exprs)
+        if node.chain == ["filters", "dateRange", "from"]:
+            compare_op_wrapper = self.compare_operations[-1]
+
+            if self.filters is None:
+                compare_op_wrapper.skip = True
+                return ast.Constant(value=True)
+
+            dateFrom = self.filters.dateRange.date_from if self.filters.dateRange else None
+            if dateFrom is not None and dateFrom != "all":
+                try:
+                    parsed_date = isoparse(dateFrom).replace(tzinfo=self.team.timezone_info)
+                except ValueError:
+                    parsed_date = relative_date_parse(dateFrom, self.team.timezone_info)
+
+                return ast.Constant(value=parsed_date)
+            else:
+                compare_op_wrapper.skip = True
+                return ast.Constant(value=True)
+        if node.chain == ["filters", "dateRange", "to"]:
+            compare_op_wrapper = self.compare_operations[-1]
+
+            if self.filters is None:
+                compare_op_wrapper.skip = True
+                return ast.Constant(value=True)
+
+            dateTo = self.filters.dateRange.date_to if self.filters.dateRange else None
+            if dateTo is not None:
+                try:
+                    parsed_date = isoparse(dateTo).replace(tzinfo=self.team.timezone_info)
+                except ValueError:
+                    parsed_date = relative_date_parse(dateTo, self.team.timezone_info)
+                return ast.Constant(value=parsed_date)
+            else:
+                compare_op_wrapper.skip = True
+                return ast.Constant(value=True)
+
         return super().visit_placeholder(node)
diff --git a/posthog/hogql/grammar/HogQLParser.g4 b/posthog/hogql/grammar/HogQLParser.g4
@@ -293,7 +293,7 @@ keywordForAlias
 alias: IDENTIFIER | keywordForAlias;  // |interval| can't be an alias, otherwise 'INTERVAL 1 SOMETHING' becomes ambiguous.
 identifier: IDENTIFIER | interval | keyword;
 enumValue: string EQ_SINGLE numberLiteral;
-placeholder: LBRACE identifier RBRACE;
+placeholder: LBRACE nestedIdentifier RBRACE;
 
 string: STRING_LITERAL | templateString;
 templateString : QUOTE_SINGLE_TEMPLATE stringContents* QUOTE_SINGLE ;

diff --git a/posthog/hogql/grammar/HogQLParser.interp b/posthog/hogql/grammar/HogQLParser.interp
diff --git a/posthog/hogql/grammar/HogQLParser.py b/posthog/hogql/grammar/HogQLParser.py
@@ -513,7 +513,7 @@ def serializedATN():
         76,0,1263,1265,3,154,77,0,1264,1261,1,0,0,0,1264,1262,1,0,0,0,1264,
         1263,1,0,0,0,1265,161,1,0,0,0,1266,1267,3,166,83,0,1267,1268,5,122,
         0,0,1268,1269,3,148,74,0,1269,163,1,0,0,0,1270,1271,5,128,0,0,1271,
-        1272,3,160,80,0,1272,1273,5,147,0,0,1273,165,1,0,0,0,1274,1277,5,
+        1272,3,134,67,0,1272,1273,5,147,0,0,1273,165,1,0,0,0,1274,1277,5,
         110,0,0,1275,1277,3,168,84,0,1276,1274,1,0,0,0,1276,1275,1,0,0,0,
         1277,167,1,0,0,0,1278,1282,5,142,0,0,1279,1281,3,170,85,0,1280,1279,
         1,0,0,0,1281,1284,1,0,0,0,1282,1280,1,0,0,0,1282,1283,1,0,0,0,1283,
@@ -9725,8 +9725,8 @@ def __init__(self, parser, parent:ParserRuleContext=None, invokingState:int=-1):
         def LBRACE(self):
             return self.getToken(HogQLParser.LBRACE, 0)
 
-        def identifier(self):
-            return self.getTypedRuleContext(HogQLParser.IdentifierContext,0)
+        def nestedIdentifier(self):
+            return self.getTypedRuleContext(HogQLParser.NestedIdentifierContext,0)
 
 
         def RBRACE(self):
@@ -9753,7 +9753,7 @@ def placeholder(self):
             self.state = 1270
             self.match(HogQLParser.LBRACE)
             self.state = 1271
-            self.identifier()
+            self.nestedIdentifier()
             self.state = 1272
             self.match(HogQLParser.RBRACE)
         except RecognitionException as re:

diff --git a/posthog/hogql/parser.py b/posthog/hogql/parser.py
@@ -1120,8 +1120,8 @@ def visitHogqlxTagAttribute(self, ctx: HogQLParser.HogqlxTagAttributeContext):
             return ast.HogQLXAttribute(name=name, value=ast.Constant(value=True))
 
     def visitPlaceholder(self, ctx: HogQLParser.PlaceholderContext):
-        name = self.visit(ctx.identifier())
-        return ast.Placeholder(field=name)
+        nested = self.visit(ctx.nestedIdentifier()) if ctx.nestedIdentifier() else []
+        return ast.Placeholder(chain=nested)
 
     def visitColumnExprTemplateString(self, ctx: HogQLParser.ColumnExprTemplateStringContext):
         return self.visit(ctx.templateString())

diff --git a/posthog/hogql/query.py b/posthog/hogql/query.py
@@ -76,9 +76,17 @@ def execute_hogql_query(
             raise ValueError(
                 f"Query contains 'filters' placeholder, yet filters are also provided as a standalone query parameter."
             )
-        if "filters" in placeholders_in_query:
+        if "filters" in placeholders_in_query or any(
+            placeholder.startswith("filters.") for placeholder in placeholders_in_query
+        ):
             select_query = replace_filters(select_query, filters, team)
-            placeholders_in_query.remove("filters")
+
+            leftover_placeholders: list[str] = []
+            for placeholder in placeholders_in_query:
+                if placeholder != "filters" and not placeholder.startswith("filters."):
+                    leftover_placeholders.append(placeholder)
+
+            placeholders_in_query = leftover_placeholders
 
         if len(placeholders_in_query) > 0:
             if len(placeholders) == 0:

diff --git a/posthog/hogql/test/_test_parser.py b/posthog/hogql/test/_test_parser.py
@@ -643,7 +643,7 @@ def test_expr_with_ignored_sql_comment(self):
         def test_placeholders(self):
             self.assertEqual(
                 self._expr("{foo}"),
-                ast.Placeholder(field="foo"),
+                ast.Placeholder(chain=["foo"]),
             )
             self.assertEqual(
                 self._expr("{foo}", {"foo": ast.Constant(value="bar")}),
@@ -854,7 +854,7 @@ def test_select_from_placeholder(self):
                 self._select("select 1 from {placeholder}"),
                 ast.SelectQuery(
                     select=[ast.Constant(value=1)],
-                    select_from=ast.JoinExpr(table=ast.Placeholder(field="placeholder")),
+                    select_from=ast.JoinExpr(table=ast.Placeholder(chain=["placeholder"])),
                 ),
             )
             self.assertEqual(
@@ -1244,7 +1244,7 @@ def test_select_placeholders(self):
                     where=ast.CompareOperation(
                         op=ast.CompareOperationOp.Eq,
                         left=ast.Constant(value=1),
-                        right=ast.Placeholder(field="hogql_val_1"),
+                        right=ast.Placeholder(chain=["hogql_val_1"]),
                     ),
                 ),
             )

diff --git a/posthog/hogql/test/test_placeholders.py b/posthog/hogql/test/test_placeholders.py
@@ -15,7 +15,7 @@ def test_replace_placeholders_simple(self):
         expr = parse_expr("{foo}")
         self.assertEqual(
             expr,
-            ast.Placeholder(field="foo", start=0, end=5),
+            ast.Placeholder(chain=["foo"], start=0, end=5),
         )
         expr2 = replace_placeholders(expr, {"foo": ast.Constant(value="bar")})
         self.assertEqual(
@@ -24,7 +24,7 @@ def test_replace_placeholders_simple(self):
         )
 
     def test_replace_placeholders_error(self):
-        expr = ast.Placeholder(field="foo")
+        expr = ast.Placeholder(chain=["foo"])
         with self.assertRaises(QueryError) as context:
             replace_placeholders(expr, {})
         self.assertEqual(
@@ -47,7 +47,7 @@ def test_replace_placeholders_comparison(self):
                 end=23,
                 op=ast.CompareOperationOp.Lt,
                 left=ast.Field(chain=["timestamp"], start=0, end=9),
-                right=ast.Placeholder(field="timestamp", start=12, end=23),
+                right=ast.Placeholder(chain=["timestamp"], start=12, end=23),
             ),
         )
         expr2 = replace_placeholders(expr, {"timestamp": ast.Constant(value=123)})
@@ -63,7 +63,7 @@ def test_replace_placeholders_comparison(self):
         )
 
     def test_assert_no_placeholders(self):
-        expr = ast.Placeholder(field="foo")
+        expr = ast.Placeholder(chain=["foo"])
         with self.assertRaises(QueryError) as context:
             replace_placeholders(expr, None)
         self.assertEqual(

diff --git a/posthog/hogql/test/test_visitor.py b/posthog/hogql/test/test_visitor.py
@@ -58,7 +58,7 @@ def test_everything_visitor(self):
                         args=[
                             ast.Alias(
                                 alias="d",
-                                expr=ast.Placeholder(field="e"),
+                                expr=ast.Placeholder(chain=["e"]),
                             ),
                             ast.OrderExpr(
                                 expr=ast.Field(chain=["c"]),

diff --git a/posthog/hogql/visitor.py b/posthog/hogql/visitor.py
@@ -493,7 +493,7 @@ def visit_placeholder(self, node: ast.Placeholder):
             start=None if self.clear_locations else node.start,
             end=None if self.clear_locations else node.end,
             type=None if self.clear_types else node.type,
-            field=node.field,
+            chain=node.chain,
         )
 
     def visit_call(self, node: ast.Call):

diff --git a/posthog/hogql_queries/error_tracking_query_runner.py b/posthog/hogql_queries/error_tracking_query_runner.py
@@ -128,7 +128,7 @@ def where(self):
                 left=ast.Field(chain=["event"]),
                 right=ast.Constant(value="$exception"),
             ),
-            ast.Placeholder(field="filters"),
+            ast.Placeholder(chain=["filters"]),
         ]
 
         if self.query.fingerprint:

diff --git a/posthog/hogql_queries/insights/trends/aggregation_operations.py b/posthog/hogql_queries/insights/trends/aggregation_operations.py
@@ -175,9 +175,9 @@ def select_aggregation(self) -> ast.Expr:
             actor = "e.distinct_id" if self.team.aggregate_users_by_distinct_id else "e.person_id"
             return parse_expr(f"count(DISTINCT {actor})")
         elif self.series.math == "weekly_active":
-            return ast.Placeholder(field="replaced")  # This gets replaced when doing query orchestration
+            return ast.Placeholder(chain=["replaced"])  # This gets replaced when doing query orchestration
         elif self.series.math == "monthly_active":
-            return ast.Placeholder(field="replaced")  # This gets replaced when doing query orchestration
+            return ast.Placeholder(chain=["replaced"])  # This gets replaced when doing query orchestration
         elif self.series.math == "unique_session":
             return parse_expr('count(DISTINCT e."$session_id")')
         elif self.series.math == "unique_group" and self.series.math_group_type_index is not None:

diff --git a/requirements.in b/requirements.in
@@ -96,7 +96,7 @@ phonenumberslite==8.13.6
 openai==1.10.0
 tiktoken==0.6.0
 nh3==0.2.14
-hogql-parser==1.0.32
+hogql-parser==1.0.35
 zxcvbn==4.4.28
 zstd==1.5.5.1
 xmlsec==1.3.13 # Do not change this version - it will break SAML

diff --git a/requirements.txt b/requirements.txt
@@ -275,7 +275,7 @@ h11==0.13.0
     #   wsproto
 hexbytes==1.0.0
     # via dlt
-hogql-parser==1.0.32
+hogql-parser==1.0.35
     # via -r requirements.in
 httpcore==1.0.2
     # via httpx