From 95fd4fb1e9bc8a68aea873e031531637e143535b Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Mon, 16 Dec 2024 15:20:13 +0000 Subject: [PATCH] =?UTF-8?q?revert(environments):=20Make=20taxonomy=20reads?= =?UTF-8?q?=20+=20writes=20project=E2=80=93based=20(#26939)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- posthog/api/property_definition.py | 21 ++++---- posthog/api/utils.py | 2 +- ...4dcdf2e218e48e0fd8a247e1b7ae0f04aee3.json} | 4 +- ...73799ecba84594ca04cfb24481cffbf6f6ca.json} | 4 +- rust/property-defs-rs/src/types.rs | 10 ++-- .../20240830124836_setup_propdefs_tables.sql | 3 -- rust/property-defs-rs/tests/updates.rs | 50 +------------------ 7 files changed, 21 insertions(+), 73 deletions(-) rename rust/property-defs-rs/.sqlx/{query-dc7d1647bdb6fcaff1b402b5ea8a376473bd31ace71b4ab6114a39b5aa141f6f.json => query-04abdef9c07ae1a30bb6f22abcfb4dcdf2e218e48e0fd8a247e1b7ae0f04aee3.json} (63%) rename rust/property-defs-rs/.sqlx/{query-ed7f5f3607948d5f5d9fca2b513822cc3ed70f0c5c14593f3d32ef6dc2ae82f7.json => query-2b9a8c4b8d323e1673d805125b4073799ecba84594ca04cfb24481cffbf6f6ca.json} (63%) diff --git a/posthog/api/property_definition.py b/posthog/api/property_definition.py index 3f4c518005a05..84b7a03f030bd 100644 --- a/posthog/api/property_definition.py +++ b/posthog/api/property_definition.py @@ -1,9 +1,8 @@ import dataclasses import json from typing import Any, Optional, cast -from django.db.models.functions import Coalesce -from django.db import connection, models +from django.db import connection from loginas.utils import is_impersonated_session from rest_framework import mixins, request, response, serializers, status, viewsets from posthog.api.utils import action @@ -100,7 +99,7 @@ class QueryContext: The raw query is used to both query and count these results """ - project_id: int + team_id: int table: str property_definition_fields: str property_definition_table: str @@ -233,7 +232,7 @@ def with_search(self, search_query: str, search_kwargs: dict) -> "QueryContext": return dataclasses.replace( self, search_query=search_query, - params={**self.params, "project_id": self.project_id, **search_kwargs}, + params={**self.params, "team_id": self.team_id, **search_kwargs}, ) def with_excluded_properties(self, excluded_properties: Optional[str], type: str) -> "QueryContext": @@ -265,7 +264,7 @@ def as_sql(self, order_by_verified: bool): SELECT {self.property_definition_fields}, {self.event_property_field} AS is_seen_on_filtered_events FROM {self.table} {self._join_on_event_property()} - WHERE coalesce({self.property_definition_table}.project_id, {self.property_definition_table}.team_id) = %(project_id)s + WHERE {self.property_definition_table}.team_id = %(team_id)s AND type = %(type)s AND coalesce(group_type_index, -1) = %(group_type_index)s {self.excluded_properties_filter} @@ -282,7 +281,7 @@ def as_count_sql(self): SELECT count(*) as full_count FROM {self.table} {self._join_on_event_property()} - WHERE coalesce({self.property_definition_table}.project_id, {self.property_definition_table}.team_id) = %(project_id)s + WHERE {self.property_definition_table}.team_id = %(team_id)s AND type = %(type)s AND coalesce(group_type_index, -1) = %(group_type_index)s {self.excluded_properties_filter} {self.name_filter} {self.numerical_filter} {self.search_query} {self.event_property_filter} {self.is_feature_flag_filter} @@ -297,7 +296,7 @@ def _join_on_event_property(self): {self.event_property_join_type} ( SELECT DISTINCT property FROM posthog_eventproperty - WHERE coalesce(project_id, team_id) = %(project_id)s {self.event_name_join_filter} + WHERE team_id = %(team_id)s {self.event_name_join_filter} ) {self.posthog_eventproperty_table_join_alias} ON {self.posthog_eventproperty_table_join_alias}.property = name """ @@ -538,7 +537,7 @@ def dangerously_get_queryset(self): query_context = ( QueryContext( - project_id=self.project_id, + team_id=self.team_id, table=( "ee_enterprisepropertydefinition FULL OUTER JOIN posthog_propertydefinition ON posthog_propertydefinition.id=ee_enterprisepropertydefinition.propertydefinition_ptr_id" if use_enterprise_taxonomy @@ -622,10 +621,8 @@ def seen_together(self, request: request.Request, *args: Any, **kwargs: Any) -> serializer = SeenTogetherQuerySerializer(data=request.GET) serializer.is_valid(raise_exception=True) - matches = EventProperty.objects.alias( - effective_project_id=Coalesce("project_id", "team_id", output_field=models.BigIntegerField()) - ).filter( - effective_project_id=self.project_id, # type: ignore + matches = EventProperty.objects.filter( + team_id=self.team_id, event__in=serializer.validated_data["event_names"], property=serializer.validated_data["property_name"], ) diff --git a/posthog/api/utils.py b/posthog/api/utils.py index 52e4cf925cc48..79c4c1028d1d2 100644 --- a/posthog/api/utils.py +++ b/posthog/api/utils.py @@ -313,7 +313,7 @@ def create_event_definitions_sql( SELECT {",".join(event_definition_fields)} FROM posthog_eventdefinition {enterprise_join} - WHERE coalesce(project_id, team_id) = %(project_id)s {conditions} + WHERE team_id = %(project_id)s {conditions} ORDER BY {additional_ordering}name ASC """ diff --git a/rust/property-defs-rs/.sqlx/query-dc7d1647bdb6fcaff1b402b5ea8a376473bd31ace71b4ab6114a39b5aa141f6f.json b/rust/property-defs-rs/.sqlx/query-04abdef9c07ae1a30bb6f22abcfb4dcdf2e218e48e0fd8a247e1b7ae0f04aee3.json similarity index 63% rename from rust/property-defs-rs/.sqlx/query-dc7d1647bdb6fcaff1b402b5ea8a376473bd31ace71b4ab6114a39b5aa141f6f.json rename to rust/property-defs-rs/.sqlx/query-04abdef9c07ae1a30bb6f22abcfb4dcdf2e218e48e0fd8a247e1b7ae0f04aee3.json index 478e535f3d462..5c8b96e695c28 100644 --- a/rust/property-defs-rs/.sqlx/query-dc7d1647bdb6fcaff1b402b5ea8a376473bd31ace71b4ab6114a39b5aa141f6f.json +++ b/rust/property-defs-rs/.sqlx/query-04abdef9c07ae1a30bb6f22abcfb4dcdf2e218e48e0fd8a247e1b7ae0f04aee3.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n INSERT INTO posthog_propertydefinition (id, name, type, group_type_index, is_numerical, volume_30_day, query_usage_30_day, team_id, project_id, property_type)\n VALUES ($1, $2, $3, $4, $5, NULL, NULL, $6, $7, $8)\n ON CONFLICT (coalesce(project_id, team_id), name, type, coalesce(group_type_index, -1))\n DO UPDATE SET property_type=EXCLUDED.property_type WHERE posthog_propertydefinition.property_type IS NULL\n ", + "query": "\n INSERT INTO posthog_propertydefinition (id, name, type, group_type_index, is_numerical, volume_30_day, query_usage_30_day, team_id, project_id, property_type)\n VALUES ($1, $2, $3, $4, $5, NULL, NULL, $6, $7, $8)\n ON CONFLICT (team_id, name, type, coalesce(group_type_index, -1))\n DO UPDATE SET property_type=EXCLUDED.property_type WHERE posthog_propertydefinition.property_type IS NULL\n ", "describe": { "columns": [], "parameters": { @@ -17,5 +17,5 @@ }, "nullable": [] }, - "hash": "dc7d1647bdb6fcaff1b402b5ea8a376473bd31ace71b4ab6114a39b5aa141f6f" + "hash": "04abdef9c07ae1a30bb6f22abcfb4dcdf2e218e48e0fd8a247e1b7ae0f04aee3" } diff --git a/rust/property-defs-rs/.sqlx/query-ed7f5f3607948d5f5d9fca2b513822cc3ed70f0c5c14593f3d32ef6dc2ae82f7.json b/rust/property-defs-rs/.sqlx/query-2b9a8c4b8d323e1673d805125b4073799ecba84594ca04cfb24481cffbf6f6ca.json similarity index 63% rename from rust/property-defs-rs/.sqlx/query-ed7f5f3607948d5f5d9fca2b513822cc3ed70f0c5c14593f3d32ef6dc2ae82f7.json rename to rust/property-defs-rs/.sqlx/query-2b9a8c4b8d323e1673d805125b4073799ecba84594ca04cfb24481cffbf6f6ca.json index 4acc68001bcdf..785a13a6d1ce7 100644 --- a/rust/property-defs-rs/.sqlx/query-ed7f5f3607948d5f5d9fca2b513822cc3ed70f0c5c14593f3d32ef6dc2ae82f7.json +++ b/rust/property-defs-rs/.sqlx/query-2b9a8c4b8d323e1673d805125b4073799ecba84594ca04cfb24481cffbf6f6ca.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n INSERT INTO posthog_eventdefinition (id, name, volume_30_day, query_usage_30_day, team_id, project_id, last_seen_at, created_at)\n VALUES ($1, $2, NULL, NULL, $3, $4, $5, NOW())\n ON CONFLICT (coalesce(project_id, team_id), name)\n DO UPDATE SET last_seen_at = $5\n ", + "query": "\n INSERT INTO posthog_eventdefinition (id, name, volume_30_day, query_usage_30_day, team_id, project_id, last_seen_at, created_at)\n VALUES ($1, $2, NULL, NULL, $3, $4, $5, NOW()) ON CONFLICT\n ON CONSTRAINT posthog_eventdefinition_team_id_name_80fa0b87_uniq\n DO UPDATE SET last_seen_at = $5\n ", "describe": { "columns": [], "parameters": { @@ -14,5 +14,5 @@ }, "nullable": [] }, - "hash": "ed7f5f3607948d5f5d9fca2b513822cc3ed70f0c5c14593f3d32ef6dc2ae82f7" + "hash": "2b9a8c4b8d323e1673d805125b4073799ecba84594ca04cfb24481cffbf6f6ca" } diff --git a/rust/property-defs-rs/src/types.rs b/rust/property-defs-rs/src/types.rs index fc3e30db09ddd..d437c62849f0e 100644 --- a/rust/property-defs-rs/src/types.rs +++ b/rust/property-defs-rs/src/types.rs @@ -175,8 +175,8 @@ impl Event { let updates = self.into_updates_inner(); if updates.len() > skip_threshold { warn!( - "Event {} for team {} has more than {} properties, skipping", - event, team_id, skip_threshold + "Event {} for team {} has more than 10,000 properties, skipping", + event, team_id ); metrics::counter!(EVENTS_SKIPPED, &[("reason", "too_many_properties")]).increment(1); return vec![]; @@ -427,8 +427,8 @@ impl EventDefinition { sqlx::query!( r#" INSERT INTO posthog_eventdefinition (id, name, volume_30_day, query_usage_30_day, team_id, project_id, last_seen_at, created_at) - VALUES ($1, $2, NULL, NULL, $3, $4, $5, NOW()) - ON CONFLICT (coalesce(project_id, team_id), name) + VALUES ($1, $2, NULL, NULL, $3, $4, $5, NOW()) ON CONFLICT + ON CONSTRAINT posthog_eventdefinition_team_id_name_80fa0b87_uniq DO UPDATE SET last_seen_at = $5 "#, Uuid::now_v7(), @@ -472,7 +472,7 @@ impl PropertyDefinition { r#" INSERT INTO posthog_propertydefinition (id, name, type, group_type_index, is_numerical, volume_30_day, query_usage_30_day, team_id, project_id, property_type) VALUES ($1, $2, $3, $4, $5, NULL, NULL, $6, $7, $8) - ON CONFLICT (coalesce(project_id, team_id), name, type, coalesce(group_type_index, -1)) + ON CONFLICT (team_id, name, type, coalesce(group_type_index, -1)) DO UPDATE SET property_type=EXCLUDED.property_type WHERE posthog_propertydefinition.property_type IS NULL "#, Uuid::now_v7(), diff --git a/rust/property-defs-rs/tests/test_migrations/20240830124836_setup_propdefs_tables.sql b/rust/property-defs-rs/tests/test_migrations/20240830124836_setup_propdefs_tables.sql index 2291e6e0d0ce3..69b8e56b9e400 100644 --- a/rust/property-defs-rs/tests/test_migrations/20240830124836_setup_propdefs_tables.sql +++ b/rust/property-defs-rs/tests/test_migrations/20240830124836_setup_propdefs_tables.sql @@ -15,7 +15,6 @@ CREATE TABLE IF NOT EXISTS posthog_eventdefinition ( CONSTRAINT posthog_eventdefinition_team_id_name_80fa0b87_uniq UNIQUE (team_id, name) ); -CREATE UNIQUE INDEX event_definition_proj_uniq ON posthog_eventdefinition (coalesce(project_id, team_id), name); CREATE TABLE IF NOT EXISTS posthog_propertydefinition ( id UUID PRIMARY KEY, @@ -32,7 +31,6 @@ CREATE TABLE IF NOT EXISTS posthog_propertydefinition ( ); CREATE UNIQUE INDEX posthog_propertydefinition_uniq ON posthog_propertydefinition (team_id, name, type, coalesce(group_type_index, -1)); -CREATE UNIQUE INDEX posthog_propdef_proj_uniq ON posthog_propertydefinition (coalesce(project_id, team_id), name, type, coalesce(group_type_index, -1)); CREATE TABLE IF NOT EXISTS posthog_eventproperty ( @@ -44,4 +42,3 @@ CREATE TABLE IF NOT EXISTS posthog_eventproperty ( ); CREATE UNIQUE INDEX posthog_event_property_unique_team_event_property ON posthog_eventproperty (team_id, event, property); -CREATE UNIQUE INDEX posthog_event_property_unique_proj_event_property ON posthog_eventproperty (coalesce(project_id, team_id), event, property); diff --git a/rust/property-defs-rs/tests/updates.rs b/rust/property-defs-rs/tests/updates.rs index 0cb3b86eea160..8e78eeed00368 100644 --- a/rust/property-defs-rs/tests/updates.rs +++ b/rust/property-defs-rs/tests/updates.rs @@ -1,9 +1,7 @@ use chrono::{DateTime, Duration, Utc}; use property_defs_rs::types::{Event, PropertyParentType, PropertyValueType}; use serde_json::json; -use sqlx::postgres::PgArguments; -use sqlx::{Arguments, PgPool}; -use uuid::Uuid; +use sqlx::PgPool; #[sqlx::test(migrations = "./tests/test_migrations")] async fn test_updates(db: PgPool) { @@ -17,6 +15,7 @@ async fn test_updates(db: PgPool) { "some_bool_as_string": "true" } "#; + let event_src = json!({ "team_id": 1, "project_id": 1, @@ -106,51 +105,6 @@ async fn test_updates(db: PgPool) { .unwrap(); } -#[sqlx::test(migrations = "./tests/test_migrations")] -async fn test_update_on_project_id_conflict(db: PgPool) { - let definition_created_at: DateTime = Utc::now() - Duration::days(1); - let mut args = PgArguments::default(); - args.add(Uuid::now_v7()).unwrap(); - args.add("foo").unwrap(); - args.add(1).unwrap(); - args.add(definition_created_at).unwrap(); - sqlx::query_with( - r#" - INSERT INTO posthog_eventdefinition (id, name, volume_30_day, query_usage_30_day, team_id, project_id, last_seen_at, created_at) - VALUES ($1, $2, NULL, NULL, $3, NULL, $4, $4) -- project_id is NULL! This definition is from before environments - "#, args - ).execute(&db).await.unwrap(); - - assert_eventdefinition_exists( - &db, - "foo", - 1, - definition_created_at, - Duration::milliseconds(0), - ) - .await - .unwrap(); - - let before = Utc::now(); - let event_src = json!({ - "team_id": 3, - "project_id": 1, - "event": "foo", - "properties": "{}" - }); - - let event = serde_json::from_value::(event_src.clone()).unwrap(); - for update in event.into_updates(10000) { - update.issue(&db).await.unwrap(); - } - - // The event def we created earlier got updated, even though it has a different `team_id`, - // because `coalesce(project_id, team_id)` matches - assert_eventdefinition_exists(&db, "foo", 1, before, Duration::seconds(1)) - .await - .unwrap(); -} - async fn assert_eventdefinition_exists( db: &PgPool, name: &str,