Skip to content

fix(cohorts): Handle string 'null' values correctly in IS_SET/IS_NOT_SET operators - #36729

Closed
haacked wants to merge 3 commits into
masterfrom
cohort-set-null-filters
Closed

haacked wants to merge 3 commits into
masterfrom
cohort-set-null-filters

Conversation

@haacked

@haacked haacked commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

Problem

Cohort filters using IS_SET and IS_NOT_SET operators have inconsistent behavior compared to other PostHog systems when handling string "null" values. Users with properties containing the literal string "null" are incorrectly included in IS_SET filters and excluded from IS_NOT_SET filters, which contradicts how feature flags and other parts of PostHog handle these values.

Fixes #29916

Changes

  • Modified /posthog/hogql/property.py: Updated _expr_to_compare_op function to handle string "null" values consistently
  • IS_SET logic: Now excludes both SQL NULL and string "null" values: (property != NULL) AND (property != 'null')
  • IS_NOT_SET logic: Now includes both SQL NULL and string "null" values: (property = NULL) OR (property = 'null') OR (NOT JSONHas(...))

Behavior Truth Table

Property Value Type IS_SET (Before) IS_SET (After) IS_NOT_SET (Before) IS_NOT_SET (After)
NULL (SQL NULL) SQL NULL ❌ FALSE ✅ FALSE ✅ TRUE ✅ TRUE
Missing key Missing ❌ FALSE ✅ FALSE ✅ TRUE ✅ TRUE
"null" (string) String ❌ TRUE (BUG) ✅ FALSE (FIXED) ❌ FALSE (BUG) ✅ TRUE (FIXED)
"" (empty string) String ✅ TRUE ✅ TRUE ✅ FALSE ✅ FALSE
"value" String ✅ TRUE ✅ TRUE ✅ FALSE ✅ FALSE
0 Number ✅ TRUE ✅ TRUE ✅ FALSE ✅ FALSE
false Boolean ✅ TRUE ✅ TRUE ✅ FALSE ✅ FALSE

Rationale for String "null" Handling

I had concerns about treating "null" as null because it could be a valid value.

However, after I looked around other code, it seems that **PostHog already treats string "null" as "not set" in other areas. For example,

Materialized Columns (existing behavior):

# From posthog/hogql/printer.py
# LEGACY_NULL_AS_NULL mode explicitly converts string "null" to SQL NULL
materialized_property_sql = f"nullIf(nullIf({column}, ''), 'null')"

I think this has to do with the fact that string "null" values can arise from:

  • Data ingestion issues where null values are serialized as strings
  • Third-party systems that represent null as string literals
  • Import processes that convert database nulls to string representations

How did you test this code?

Automated Testing

  • Unit Tests
  • Manual tests

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

haacked and others added 3 commits August 15, 2025 10:38
…SET operators

String "null" values were incorrectly treated as "set" in cohort filters, causing
inconsistency with regular property filters. This fix ensures both operators
handle string "null" values correctly:

- IS_SET: Now excludes both SQL NULL and string "null" values
- IS_NOT_SET: Now includes both SQL NULL and string "null" values

Fixes #29916
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: In Cohorts, the 'set' operator filters in null values for person properties

1 participant