Skip to content

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

Closed

Description

@ordehi

Bug description

In Cohorts, 'set' operator filters in null values for person properties.

Expected outcome

  • 'set' should filter out null values
  • 'not set' should include null values

Using Have the property vs Does not have the property doesn't solve this because we want to filter out users who have the property but with a null value.

In filters, 'not set' filters in 'null while 'set' filters it out. This means there's an inconsistency with how we handle this in Cohorts.

How to reproduce

  1. Create a cohort and filter by having a person property that you know some users have null for and select 'set'
  2. notice the list of users includes users with 'null'

You can test with this cohort: https://us.posthog.com/project/53137/cohorts/122367

Additional context

From: https://posthoghelp.zendesk.com/agent/tickets/27021 (moved to PostHog: https://us.posthog.com/project/2/support/tickets/29216)

Debug info

null

Activity

  1. moved this from Todo to No status in Feature Flagson Aug 6, 2025
  2. self-assigned this
    on Aug 15, 2025
  3. moved this from Todo to In Progress in Feature Flagson Aug 15, 2025
  4. added a commit that references this issue on Aug 15, 2025
    24dafee
  5. haacked commented on Aug 15, 2025

    @haacked
    Contributor

    In filters, 'not set' filters in 'null while 'set' filters it out. This means there's an inconsistency with how we handle this in Cohorts.

    Just to clarify, you mean NOT_SET filters include null values while SET excludes null values, correct? What about the string "null" value? How do they work with that?

  6. 11 remaining items

  7. Lakshya77089 commented on Jun 30, 2026

    @Lakshya77089

    Spent a bit of time digging into this and figured I'd write up what I found, because there's already an open PR (#66573) and a bunch of earlier ones that got closed (#58501, #53250, #36729, #59962, #65069). When that many attempts bounce, it usually means the actual SQL change isn't the hard part.

    Two things that I think are tripping people up:

    First, is_set/is_not_set is implemented in two different places. The legacy one is prop_filter_json_extract() in posthog/models/property/util.py, where is_set is just JSONHas(...) — and JSONHas happily returns true for a key whose value is JSON null, which is exactly the bug here. That's the path #66573 fixes. But there's a second one in _expr_to_compare_op() in posthog/hogql/property.py, where is_set becomes expr != NULL. The newer cohort query builder (hogql_cohort_query.py) sends person property filters through that HogQL path. So depending on how a cohort actually gets calculated, you can hit either implementation, and patching only one leaves the other behaving differently. Might be worth confirming which path the repro cohort (122367) goes through before merging anything.

    Second, on the HogQL side the property read also goes through the scrubbing in clickhouse_property_resolution.py (nullIf(nullIf(col, ''), 'null')), and that depends on whether the property is materialized. So a materialized property and a plain JSON one can give different answers for the same data — which probably explains why this shows up on Cloud but is hard to reproduce on a clean local instance.

    The other thing is the semantics still aren't fully nailed down. The issue covers key-absent and JSON-null, but the string "null" case (the one @haacked asked about, and what #36729 was specifically about) keeps coming up and never really got settled. Whatever the answer is, it needs to be applied to both paths with tests for all three cases — the denormalized branch in particular got flagged as under-tested on #65069 and #66573.

    I don't have a Cloud/ClickHouse setup to verify query output myself, so I'm not going to throw another PR on top of #66573 — just wanted to flag the two-path thing so it doesn't get half-fixed again. Happy to help once the string "null" behavior is decided.

  8. moved this from Todo to In Progress in Elixpoo Operationson Aug 9, 2026
  9. added a commit that references this issue on Aug 16, 2026
    ea558cb
  10. darkopia commented on Nov 6, 2026

    @darkopia
    Contributor

    Closing because the cohort 'is set' operator no longer includes people whose value is null. Add a comment if this was wrong.

  11. moved this from In Progress to Done in Elixpoo Operationson Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions