feat(flags): warn when SQL reads $feature_flag_called from events

frontend/snapshots.yml

@@ -7399,7 +7399,7 @@ snapshots:
     products-posthog-ai-virtualizedthread--anchored-open--light:
         hash: v1.k794b7964.1b0387275217fb820cbbe4a18caa5cde4be9521d4e296933e80e8f78a9914490.3TrbyabUGN-G6zrsTie8aaMDWrYs1wWMW7OIANOjsbE
     products-posthog-ai-virtualizedthread--anchored-open-mid-turn--dark:
-        hash: v1.k794b7964.eecc3dde9e8475fb8f20f259880edae96e2750529897fc326c44d33ce20e0885.jUSUfoNAuVJvfWe8M9IRRg_CirdzkCKjQjoVsutO9vY
+        hash: v1.k794b7964.f3f7ac7d6170d27474d9ca75b76407b3a3ff438bae0f350a05f31bf77f9f441a.MsZ1cBXcrgbiyZj0dGOZhBF4sZ8QCCQuznSJuUhPqOY
     products-posthog-ai-virtualizedthread--anchored-open-mid-turn--light:
         hash: v1.k794b7964.1b0387275217fb820cbbe4a18caa5cde4be9521d4e296933e80e8f78a9914490.qUuhFOn4BwbL8REUimYFdxCpOmg64Cj1HJD2Ovwdan0
     products-posthog-ai-virtualizedthread--anchored-open-mid-turn-long-tail--dark:

posthog/hogql/flag_called_warnings.py

@@ -0,0 +1,95 @@
+from posthog.schema import HogQLNotice
+
+from posthog.hogql import ast
+from posthog.hogql.context import HogQLContext
+from posthog.hogql.database.schema.events import EventsTable
+from posthog.hogql.resolver import resolve_types
+from posthog.hogql.visitor import TraversingVisitor, clone_expr
+
+# posthog/models/flag_evaluations/sql.py defines this name too. It imports django.conf, which this module keeps
+# off its import path, so the name is spelled out here.
+FLAG_CALLED_EVENT = "$feature_flag_called"
+
+# Orgs that can't query posthog.flag_evaluations yet get this warning too, so the copy speaks of the move in the future.
+FLAG_CALLED_ON_EVENTS_WARNING = (
+    f"{FLAG_CALLED_EVENT} is moving from events to posthog.flag_evaluations. Once the move finishes "
+    "for your organization, this query will stop returning these events and you can query "
+    "posthog.flag_evaluations instead."
+)
+
+
+class _FlagCalledOnEventsFinder(TraversingVisitor):
+    def __init__(self) -> None:
+        super().__init__()
+        self.literals: list[ast.Constant] = []
+
+    def visit_select_query(self, node: ast.SelectQuery) -> None:
+        # The resolver inlines a saved view's body, parsed from the view's own text. Its offsets do not
+        # point into the user's query.
+        if node.view_name is not None:
+            return
+        super().visit_select_query(node)
+
+    def visit_alias(self, node: ast.Alias) -> None:
+        # The resolver inlines a saved expression's body in a hidden alias. The body's offsets point into its own text.
+        if node.hidden:
+            return
+        super().visit_alias(node)
+
+    def visit_field(self, node: ast.Field) -> None:
+        # The inherited visit follows the field's type into shared CTE types, which takes exponential time.
+        pass
+
+    def visit_compare_operation(self, node: ast.CompareOperation) -> None:
+        if node.op in (ast.CompareOperationOp.Eq, ast.CompareOperationOp.In, ast.CompareOperationOp.GlobalIn):
+            for field_side, value_side in ((node.left, node.right), (node.right, node.left)):
+                if _is_events_event_field(field_side):
+                    self.literals.extend(_flag_called_constants(value_side))
+        super().visit_compare_operation(node)
+
+
+def _is_events_event_field(expr: ast.Expr) -> bool:
+    while isinstance(expr, ast.Alias):
+        expr = expr.expr
+    if not isinstance(expr, ast.Field):
+        return False
+    # A select alias used in WHERE wraps the column in one FieldAliasType per alias.
+    field_type = expr.type
+    while isinstance(field_type, ast.FieldAliasType):
+        field_type = field_type.type
+    if not isinstance(field_type, ast.FieldType) or field_type.name != "event":
+        return False
+    table_type = field_type.table_type
+    while isinstance(table_type, (ast.TableAliasType, ast.ColumnAliasedTableType)):
+        table_type = table_type.table_type
+    return isinstance(table_type, ast.TableType) and isinstance(table_type.table, EventsTable)
+
+
+def _flag_called_constants(expr: ast.Expr) -> list[ast.Constant]:
+    if isinstance(expr, ast.Constant):
+        return [expr] if expr.value == FLAG_CALLED_EVENT else []
+    if isinstance(expr, (ast.Tuple, ast.Array)):
+        return [constant for sub in expr.exprs for constant in _flag_called_constants(sub)]
+    return []
+
+
+def flag_called_on_events_warnings(
+    node: ast.SelectQuery | ast.SelectSetQuery, context: HogQLContext
+) -> list[HogQLNotice]:
+    """A warning on each `$feature_flag_called` literal that a query compares to the events table's `event` column.
+
+    The query resolves on a clone, because the printed AST has been rewritten and its offsets no longer match
+    what the user typed.
+    """
+    # A caller that passes an already printed AST never builds the database, and its AST may be rewritten.
+    if context.database is None:
+        return []
+    finder = _FlagCalledOnEventsFinder()
+    with context.timings.measure("flag_called_warnings"):
+        finder.visit(resolve_types(clone_expr(node), context, dialect="clickhouse"))
+    # A literal that a variable supplies has no span, and a notice without one marks the whole query.
+    return [
+        HogQLNotice(message=FLAG_CALLED_ON_EVENTS_WARNING, start=literal.start, end=literal.end)
+        for literal in finder.literals
+        if literal.start is not None and literal.end is not None
+    ]

posthog/hogql/metadata.py

@@ -32,6 +32,7 @@
     NotImplementedError as HogQLNotImplementedError,
 )
 from posthog.hogql.filters import replace_filters
+from posthog.hogql.flag_called_warnings import FLAG_CALLED_EVENT, flag_called_on_events_warnings
 from posthog.hogql.index_eligibility import IndexEligibilityReport, build_index_eligibility_report
 from posthog.hogql.metadata_heuristics import run_metadata_heuristics
 from posthog.hogql.modifiers import create_default_modifiers_for_team
@@ -84,7 +85,7 @@
     return columns
 
 
 def get_hogql_metadata(
     query: HogQLMetadata,
     team: Team,
     user: Optional[User] = None,
@@ -212,6 +213,9 @@
             if prepared_ast:
                 response.ch_table_names = get_table_names(prepared_ast)
 
+            if source is None and FLAG_CALLED_EVENT in query.query:
+                heuristic_warnings.extend(_flag_called_on_events_warnings(hogql_ast, context, team))
+
             if source is None and query.indexUsage and _index_usage_enabled(team):
                 _attach_index_usage(response, hogql_ast, context)
         else:
@@ -282,6 +286,37 @@
     )
 
 
+def _flag_called_move_notices_enabled(team: Team) -> bool:
+    """Customers have not been told yet that $feature_flag_called is moving to posthog.flag_evaluations.
+
+    A warning about the move with no announcement behind it reads as a bug, so the flag stays off until the
+    announcement goes out.
+    """
+    return feature_enabled_or_false(
+        "flag-called-move-notices",
+        str(team.uuid),
+        groups={"organization": str(team.organization_id), "project": str(team.id)},
+        group_properties={
+            "organization": {"id": str(team.organization_id)},
+            "project": {"id": str(team.id)},
+        },
+    )
+
+
+def _flag_called_on_events_warnings(
+    hogql_ast: Union[ast.SelectQuery, ast.SelectSetQuery], context: HogQLContext, team: Team
+) -> list[HogQLNotice]:
+    try:
+        if not _flag_called_move_notices_enabled(team):
+            return []
+        return flag_called_on_events_warnings(hogql_ast, context)
+    except Exception:
+        # The warning is advisory. A query that compiles must not be reported as invalid because this
+        # check failed, and the caller turns any exception here into an invalid query.
+        logger.exception("hogql_flag_called_warning_failed", team_id=context.team_id)
+        return []
+
+
 def _attach_index_usage(
     response: HogQLMetadataResponse,
     hogql_ast: Union[ast.SelectQuery, ast.SelectSetQuery],

posthog/hogql/test/test_metadata.py

@@ -22,15 +22,19 @@
 )
 
 from posthog.hogql.direct_connection import INVALID_CONNECTION_ID_ERROR
+from posthog.hogql.flag_called_warnings import FLAG_CALLED_ON_EVENTS_WARNING
 from posthog.hogql.metadata import get_hogql_metadata
 from posthog.hogql.parser import parse_select
 from posthog.hogql.taxonomy_validation import MAX_SUGGESTED_NAMES
 
 from posthog.api.services.query import process_query_model
 from posthog.models import EventDefinition, PropertyDefinition, Team
+from posthog.models.scoping import team_scope
 from posthog.taxonomy.dynamic_properties import DYNAMIC_PROPERTY_PATTERNS
 
 from products.cohorts.backend.models.cohort import Cohort
+from products.data_modeling.backend.facade.models import DataWarehouseSavedQuery
+from products.data_tools.backend.models.expression import DataWarehouseExpression
 from products.product_analytics.backend.facade.models import InsightVariable
 from products.warehouse_sources.backend.facade.models import DataWarehouseTable, ExternalDataSchema, ExternalDataSource
 from products.warehouse_sources.backend.facade.types import ExternalDataSourceType
@@ -355,6 +359,76 @@ def test_metadata_does_not_warn_for_known_event_literal(self):
         self.assertTrue(metadata.isValid)
         self.assertEqual(metadata.warnings, [])
 
+    @parameterized.expand(
+        [
+            ("equals", "SELECT count() FROM events WHERE event = '$feature_flag_called'", True, 1),
+            ("in_list", "SELECT count() FROM events WHERE event IN ('$pageview', '$feature_flag_called')", True, 1),
+            ("namespaced_table", "SELECT count() FROM posthog.events WHERE event = '$feature_flag_called'", True, 1),
+            ("aliased_table", "SELECT count() FROM events AS e WHERE e.event = '$feature_flag_called'", True, 1),
+            ("aliased_column", "SELECT event AS name FROM events WHERE name = '$feature_flag_called'", True, 1),
+            ("table_not_available", "SELECT count() FROM events WHERE event = '$feature_flag_called'", False, 1),
+            (
+                "flag_evaluations_table",
+                "SELECT count() FROM posthog.flag_evaluations WHERE event = '$feature_flag_called'",
+                True,
+                0,
+            ),
+            ("saved_view_body", "SELECT count() FROM flag_calls_view WHERE event != '$feature_flag_called'", True, 0),
+            (
+                "property_named_event",
+                "SELECT count() FROM events WHERE properties.event = '$feature_flag_called'",
+                True,
+                0,
+            ),
+            ("not_compared_to_event", "SELECT '$feature_flag_called' FROM events", True, 0),
+            (
+                "saved_expression_body",
+                "SELECT count() FROM events WHERE flag_call_expr AND distinct_id != '$feature_flag_called'",
+                True,
+                0,
+            ),
+            ("move_notices_off", "SELECT count() FROM events WHERE event = '$feature_flag_called'", True, 0, False),
+        ]
+    )
+    def test_metadata_warns_for_flag_called_read_from_events(
+        self,
+        _name: str,
+        query: str,
+        flag_evaluations_enabled: bool,
+        expected: int,
+        move_notices_enabled: bool = True,
+    ) -> None:
+        DataWarehouseSavedQuery.objects.create(
+            team=self.team,
+            name="flag_calls_view",
+            query={"query": "SELECT uuid, event FROM events WHERE event = '$feature_flag_called'"},
+            columns={"uuid": "String", "event": "String"},
+        )
+        with team_scope(self.team.pk, canonical=True):
+            DataWarehouseExpression.objects.create(
+                team=self.team,
+                table_name="events",
+                field_name="flag_call_expr",
+                expression="event = '$feature_flag_called'",
+            )
+
+        with (
+            patch(
+                "products.feature_flags.backend.facade.flags.is_flag_evaluations_table_enabled",
+                return_value=flag_evaluations_enabled,
+            ),
+            patch("posthog.hogql.metadata.feature_enabled_or_false", return_value=move_notices_enabled),
+        ):
+            metadata = self._select(query)
+
+        self.assertTrue(metadata.isValid, metadata.errors)
+        warnings = [w for w in metadata.warnings if w.message == FLAG_CALLED_ON_EVENTS_WARNING]
+        literal_start = query.rindex("'$feature_flag_called'")
+        self.assertEqual(
+            [(w.start, w.end, w.fix) for w in warnings],
+            [(literal_start, literal_start + len("'$feature_flag_called'"), None)] * expected,
+        )
+
     def test_metadata_warns_for_unknown_event_in_literal(self):
         EventDefinition.objects.create(team=self.team, name="signed_up")