Skip to content

Commit 78cbd1a

Browse files
fix(translator): % Split without explicitly provided identity context translate to FALSE (#16)
Co-authored-by: Matthew Elwell <mjelwell89@gmail.com>
1 parent cfc3d4e commit 78cbd1a

3 files changed

Lines changed: 21 additions & 24 deletions

File tree

src/flagsmith_sql_flag_engine/translator.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -435,19 +435,13 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
435435
identity: dict[str, object] = ctx.evaluation_context.get("identity") or {} # type: ignore[assignment]
436436
kind = classification.kind
437437
if not prop:
438-
# Implicit `$.identity.key` — engine returns False when no
439-
# identity, or when the identity lacks `key`. The engine
440-
# never synthesises one from env+identifier.
441-
if not identity.get("key"):
442-
return "FALSE"
438+
# In traditional engine implementations, this branch implies
439+
# an identity-less context, which makes no sense for the SQL engine.
440+
# Assume identity key.
443441
value_expr = ctx.dialect.cast_string(ctx.identity_key_expr)
444442
elif kind == "key":
445-
if not identity.get("key"):
446-
return "FALSE"
447443
value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.key"))
448444
elif kind == "identifier":
449-
if not identity.get("identifier"):
450-
return "FALSE"
451445
value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.identifier"))
452446
elif kind == "identity_object":
453447
# PERCENTAGE_SPLIT on `$.identity` — the whole dict. Engine

tests/harnesses/clickhouse.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@
3939
# condition, so we accept the divergence on this niche shape
4040
# (`$.`-prefixed trait names) and let callers fall back to the engine.
4141
"test_jsonpath_like_trait__existing_jsonpath__should_match_trait",
42+
# Does not apply to SQL engine implementation; we always have access to
43+
# identity key.
44+
"test_percentage_split__no_identity_key__should_match",
4245
}
4346

4447

tests/test_translator_unit.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -542,9 +542,10 @@ def _ctx_identity_without(field: str) -> TranslateContext:
542542
)
543543

544544

545-
def test_translate_segment__percentage_split_implicit_key_no_identity__compiles_to_false() -> None:
546-
# Given a PERCENTAGE_SPLIT with no property (implicit `$.identity.key`) and an eval
547-
# context with no identity
545+
def test_translate_segment__percentage_split_implicit_key_no_identity__inlines_md5() -> None:
546+
# Given a PERCENTAGE_SPLIT with no property (implicit `$.identity.key`) and an
547+
# eval context with no identity — the row-oriented case (e.g. segment-membership
548+
# counting over the whole IDENTITIES table)
548549
seg: SegmentContext = {
549550
"key": "ps2",
550551
"name": "s",
@@ -556,12 +557,12 @@ def test_translate_segment__percentage_split_implicit_key_no_identity__compiles_
556557
],
557558
}
558559

559-
# When / Then the predicate collapses to FALSE — engine returns False without identity
560-
assert translate_segment(seg, _ctx_no_identity()) == "((FALSE))"
560+
# When / Then it hashes the per-row identity key rather than folding to FALSE
561+
assert translate_segment(seg, _ctx_no_identity()) != "((FALSE))"
561562

562563

563-
def test_translate_segment__percentage_split_identity_key_missing__compiles_to_false() -> None:
564-
# Given the eval context's identity has no `key`
564+
def test_translate_segment__percentage_split_key_no_context_identity__inlines_md5() -> None:
565+
# Given a PERCENTAGE_SPLIT on `$.identity.key` and a context identity without `key`
565566
seg: SegmentContext = {
566567
"key": "ps3",
567568
"name": "s",
@@ -575,14 +576,13 @@ def test_translate_segment__percentage_split_identity_key_missing__compiles_to_f
575576
],
576577
}
577578

578-
# When / Then the predicate collapses to FALSE
579-
assert translate_segment(seg, _ctx_identity_without("key")) == "((FALSE))"
579+
# When / Then it still hashes the per-row identity key rather than folding to FALSE
580+
assert translate_segment(seg, _ctx_identity_without("key")) != "((FALSE))"
580581

581582

582-
def test_translate_segment__percentage_split_identity_identifier_missing__compiles_to_false() -> (
583-
None
584-
):
585-
# Given the eval context's identity has no `identifier`
583+
def test_translate_segment__percentage_split_identifier_no_context_identity__inlines_md5() -> None:
584+
# Given a PERCENTAGE_SPLIT on `$.identity.identifier` and a context identity
585+
# without `identifier`
586586
seg: SegmentContext = {
587587
"key": "ps4",
588588
"name": "s",
@@ -600,8 +600,8 @@ def test_translate_segment__percentage_split_identity_identifier_missing__compil
600600
],
601601
}
602602

603-
# When / Then the predicate collapses to FALSE
604-
assert translate_segment(seg, _ctx_identity_without("identifier")) == "((FALSE))"
603+
# When / Then it still hashes the per-row identifier rather than folding to FALSE
604+
assert translate_segment(seg, _ctx_identity_without("identifier")) != "((FALSE))"
605605

606606

607607
def test_translate_segment__percentage_split_unknown_jsonpath__returns_none() -> None:

0 commit comments

Comments
 (0)