diff --git a/src/flagsmith_sql_flag_engine/translator.py b/src/flagsmith_sql_flag_engine/translator.py index c836120..a49ff87 100644 --- a/src/flagsmith_sql_flag_engine/translator.py +++ b/src/flagsmith_sql_flag_engine/translator.py @@ -435,19 +435,13 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str | identity: dict[str, object] = ctx.evaluation_context.get("identity") or {} # type: ignore[assignment] kind = classification.kind if not prop: - # Implicit `$.identity.key` — engine returns False when no - # identity, or when the identity lacks `key`. The engine - # never synthesises one from env+identifier. - if not identity.get("key"): - return "FALSE" + # In traditional engine implementations, this branch implies + # an identity-less context, which makes no sense for the SQL engine. + # Assume identity key. value_expr = ctx.dialect.cast_string(ctx.identity_key_expr) elif kind == "key": - if not identity.get("key"): - return "FALSE" value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.key")) elif kind == "identifier": - if not identity.get("identifier"): - return "FALSE" value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.identifier")) elif kind == "identity_object": # PERCENTAGE_SPLIT on `$.identity` — the whole dict. Engine diff --git a/tests/harnesses/clickhouse.py b/tests/harnesses/clickhouse.py index 2378a3c..0753299 100644 --- a/tests/harnesses/clickhouse.py +++ b/tests/harnesses/clickhouse.py @@ -39,6 +39,9 @@ # condition, so we accept the divergence on this niche shape # (`$.`-prefixed trait names) and let callers fall back to the engine. "test_jsonpath_like_trait__existing_jsonpath__should_match_trait", + # Does not apply to SQL engine implementation; we always have access to + # identity key. + "test_percentage_split__no_identity_key__should_match", } diff --git a/tests/test_translator_unit.py b/tests/test_translator_unit.py index 946b5c5..df12a6c 100644 --- a/tests/test_translator_unit.py +++ b/tests/test_translator_unit.py @@ -542,9 +542,10 @@ def _ctx_identity_without(field: str) -> TranslateContext: ) -def test_translate_segment__percentage_split_implicit_key_no_identity__compiles_to_false() -> None: - # Given a PERCENTAGE_SPLIT with no property (implicit `$.identity.key`) and an eval - # context with no identity +def test_translate_segment__percentage_split_implicit_key_no_identity__inlines_md5() -> None: + # Given a PERCENTAGE_SPLIT with no property (implicit `$.identity.key`) and an + # eval context with no identity — the row-oriented case (e.g. segment-membership + # counting over the whole IDENTITIES table) seg: SegmentContext = { "key": "ps2", "name": "s", @@ -556,12 +557,12 @@ def test_translate_segment__percentage_split_implicit_key_no_identity__compiles_ ], } - # When / Then the predicate collapses to FALSE — engine returns False without identity - assert translate_segment(seg, _ctx_no_identity()) == "((FALSE))" + # When / Then it hashes the per-row identity key rather than folding to FALSE + assert translate_segment(seg, _ctx_no_identity()) != "((FALSE))" -def test_translate_segment__percentage_split_identity_key_missing__compiles_to_false() -> None: - # Given the eval context's identity has no `key` +def test_translate_segment__percentage_split_key_no_context_identity__inlines_md5() -> None: + # Given a PERCENTAGE_SPLIT on `$.identity.key` and a context identity without `key` seg: SegmentContext = { "key": "ps3", "name": "s", @@ -575,14 +576,13 @@ def test_translate_segment__percentage_split_identity_key_missing__compiles_to_f ], } - # When / Then the predicate collapses to FALSE - assert translate_segment(seg, _ctx_identity_without("key")) == "((FALSE))" + # When / Then it still hashes the per-row identity key rather than folding to FALSE + assert translate_segment(seg, _ctx_identity_without("key")) != "((FALSE))" -def test_translate_segment__percentage_split_identity_identifier_missing__compiles_to_false() -> ( - None -): - # Given the eval context's identity has no `identifier` +def test_translate_segment__percentage_split_identifier_no_context_identity__inlines_md5() -> None: + # Given a PERCENTAGE_SPLIT on `$.identity.identifier` and a context identity + # without `identifier` seg: SegmentContext = { "key": "ps4", "name": "s", @@ -600,8 +600,8 @@ def test_translate_segment__percentage_split_identity_identifier_missing__compil ], } - # When / Then the predicate collapses to FALSE - assert translate_segment(seg, _ctx_identity_without("identifier")) == "((FALSE))" + # When / Then it still hashes the per-row identifier rather than folding to FALSE + assert translate_segment(seg, _ctx_identity_without("identifier")) != "((FALSE))" def test_translate_segment__percentage_split_unknown_jsonpath__returns_none() -> None: