From 9efa2a0a93ff2c834518512f9c709eafc3d8bfd2 Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Tue, 1 Oct 2024 19:58:34 +0300 Subject: [PATCH] fix(propdefs): Handle unresolvable group-type indexes properly (#25315) --- rust/property-defs-rs/src/config.rs | 6 ------ rust/property-defs-rs/src/types.rs | 10 +++++++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/rust/property-defs-rs/src/config.rs b/rust/property-defs-rs/src/config.rs index 6e664bd513a58..f343a61556624 100644 --- a/rust/property-defs-rs/src/config.rs +++ b/rust/property-defs-rs/src/config.rs @@ -41,12 +41,6 @@ pub struct Config { #[envconfig(default = "1000000")] pub cache_capacity: usize, - // We expire cache entries after this many seconds. This is /mostly/ to handle - // cases where we don't handle an insert error properly, so that a subsequent - // event seen can re-try the insert. - #[envconfig(default = "600")] // 10 minutes - pub cache_ttl_seconds: u64, - // We impose a slow-start, where each batch update operation is delayed by // this many milliseconds, multiplied by the % of the cache currently unused. The idea // is that we want to drip-feed updates to the DB during warmup, since diff --git a/rust/property-defs-rs/src/types.rs b/rust/property-defs-rs/src/types.rs index 43a3cc3b3b6d8..892ddcc4b5534 100644 --- a/rust/property-defs-rs/src/types.rs +++ b/rust/property-defs-rs/src/types.rs @@ -441,7 +441,7 @@ impl PropertyDefinition { Some(GroupType::Resolved(_, i)) => Some(*i as i16), Some(GroupType::Unresolved(group_name)) => { warn!( - "Group type {} not resolved for property definition {} for team {}, skipping", + "Group type {} not resolved for property definition {} for team {}, skipping update", group_name, self.name, self.team_id ); None @@ -452,6 +452,14 @@ impl PropertyDefinition { } }; + if group_type_index.is_none() && matches!(self.event_type, PropertyParentType::Group) { + // Some teams/users wildly misuse group-types, and if we fail to issue an update + // during the transaction (which we do if we don't have a group-type index for a + // group property), the entire transaction is aborted, so instead we just warn + // loudly about this (above, and at resolve time), and drop the update. + return Ok(()); + } + sqlx::query!( r#" INSERT INTO posthog_propertydefinition (id, name, type, group_type_index, is_numerical, volume_30_day, query_usage_30_day, team_id, property_type)