Skip to content

Commit 1cf1be7

Browse files
committed
Move type-aware trait predicates to the dialect
Translator was emitting Snowflake-specific SQL — TYPEOF(), IS_BOOLEAN(), IS_DECIMAL(), TRY_TO_DOUBLE(), and ::FLOAT casts — directly from _trait_typed_eq / _trait_typed_in, which broke the dialect-agnostic abstraction. Moved both functions onto SnowflakeDialect as `trait_eq` and `trait_in`, added the corresponding methods to the Dialect protocol, and have the translator delegate. Translator now only knows that traits live behind dialect.trait_path() and that EQUAL/NOT_EQUAL/IN need a type-aware predicate that the dialect emits — no leaking knowledge of how Snowflake stores or discriminates trait types. The remaining VARIANT mentions in translator.py are clearly labelled "Snowflake's implementation" rather than implied invariants. Engine-faithful IN value parsing (`_engine_in_values`) stays in the translator since it mirrors flag_engine's parsing rules and is purely Python-side, dialect-independent. The translator hands the parsed list to dialect.trait_in so the dialect doesn't redo the parsing. Parity: 656 passed / 2 xfailed. Coverage: 100% line + branch.
1 parent cfedda4 commit 1cf1be7

3 files changed

Lines changed: 103 additions & 96 deletions

File tree

src/flagsmith_sql_flag_engine/dialect.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,23 @@ def trait_path(self, alias: str, trait_key: str) -> str:
4444
"""
4545
...
4646

47+
def trait_eq(self, alias: str, trait_key: str, value: object, negate: bool) -> str:
48+
"""Type-aware EQUAL / NOT_EQUAL predicate on a trait, mirroring
49+
`flag_engine`'s per-type coercion (segment value cast to the
50+
trait's runtime type before compare; cast failure → no match for
51+
both ops). Implementation is dialect-specific because trait-type
52+
discrimination and runtime type-coercion casts both vary by engine.
53+
"""
54+
...
55+
56+
def trait_in(self, alias: str, trait_key: str, items: list[str]) -> str:
57+
"""Type-aware IN predicate on a trait, mirroring engine semantics:
58+
string trait does direct lookup; integer trait stringifies and
59+
looks up; other trait types never match. `items` is the parsed
60+
candidate list per `flag_engine`'s `_get_in_values`.
61+
"""
62+
...
63+
4764
# --- string operations ---
4865

4966
def position(self, needle_lit: str, haystack_expr: str) -> str:

src/flagsmith_sql_flag_engine/dialects/snowflake.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939

4040
from __future__ import annotations
4141

42+
from flagsmith_sql_flag_engine.utils import string_literal
43+
4244
# Canonical schema the translator expects. Run this once when standing up
4345
# a Snowflake-backed Flagsmith installation; the CDC pipeline that
4446
# materialises IDENTITIES from the source-of-truth feed populates the
@@ -89,6 +91,70 @@ def trait_path(self, alias: str, trait_key: str) -> str:
8991
escaped = trait_key.replace('"', '""')
9092
return f'{alias}.traits:"{escaped}"'
9193

94+
def trait_eq(self, alias: str, trait_key: str, value: object, negate: bool) -> str:
95+
path = self.trait_path(alias, trait_key)
96+
str_path = self.cast_string(path)
97+
str_value = str(value)
98+
str_lit = string_literal(str_value)
99+
# Engine bool cast: `lambda v: v not in ("False", "false")`. We compare
100+
# against the variant's `::STRING` form ('true'/'false') rather than
101+
# invoke `(...)::BOOLEAN` directly — Snowflake's optimiser eagerly
102+
# evaluates the BOOLEAN cast even when the IS_BOOLEAN guard would
103+
# have short-circuited, and a non-bool variant blows up the query
104+
# (`100037: Boolean value 'red' is not recognized`).
105+
bool_str_lit = "'false'" if str_value in ("False", "false") else "'true'"
106+
# Engine int/float cast: int(v) / float(v); ValueError → no match.
107+
try:
108+
int_lit: str | None = str(int(str_value))
109+
except (ValueError, TypeError):
110+
int_lit = None
111+
try:
112+
float_lit: str | None = repr(float(str_value))
113+
except (ValueError, TypeError):
114+
float_lit = None
115+
116+
if not negate:
117+
# Fast string compare always present — handles VARCHAR traits and
118+
# canonically-stringified INTEGER traits in one cheap branch.
119+
clauses = [f"{str_path} = {str_lit}"]
120+
clauses.append(f"(IS_BOOLEAN({path}) AND {str_path} = {bool_str_lit})")
121+
if float_lit is not None:
122+
# Variant float `1.23` stringifies to `'1.230000000000000e+00'`-ish
123+
# in Snowflake — direct string compare misses it, so a typed
124+
# branch is needed. TRY_TO_DOUBLE on the string form sidesteps
125+
# the same eager-eval trap as the bool branch.
126+
clauses.append(
127+
f"((IS_DECIMAL({path}) OR IS_DOUBLE({path}))"
128+
f" AND TRY_TO_DOUBLE({str_path}) = {float_lit})"
129+
)
130+
return "(" + " OR ".join(clauses) + ")"
131+
132+
# NOT_EQUAL: per-type dispatch — engine returns True only when the
133+
# cast succeeded *and* values differ, which an OR-of-positives
134+
# can't express without over-matching.
135+
no_match = "FALSE" # engine returns False on cast failure
136+
bool_branch = f"{str_path} <> {bool_str_lit}"
137+
int_branch = f"({path})::NUMBER <> {int_lit}" if int_lit is not None else no_match
138+
float_branch = f"({path})::FLOAT <> {float_lit}" if float_lit is not None else no_match
139+
return (
140+
f"((TYPEOF({path}) = 'BOOLEAN' AND {bool_branch})"
141+
f" OR (TYPEOF({path}) = 'INTEGER' AND {int_branch})"
142+
f" OR (TYPEOF({path}) IN ('DECIMAL', 'DOUBLE') AND {float_branch})"
143+
f" OR (TYPEOF({path}) NOT IN ('BOOLEAN', 'INTEGER', 'DECIMAL', 'DOUBLE')"
144+
f" AND {str_path} <> {str_lit}))"
145+
)
146+
147+
def trait_in(self, alias: str, trait_key: str, items: list[str]) -> str:
148+
# Collapsed to a single `TYPEOF` gate around one string IN compare —
149+
# Snowflake stringifies INTEGER variants without decimals, so the same
150+
# `(path)::STRING IN (...)` works for both VARCHAR and INTEGER. Bool /
151+
# float / array traits never match per engine semantics, so they fall
152+
# outside the gate.
153+
path = self.trait_path(alias, trait_key)
154+
str_path = self.cast_string(path)
155+
item_lits = ",".join(string_literal(v) for v in items)
156+
return f"(TYPEOF({path}) IN ('VARCHAR', 'INTEGER') AND {str_path} IN ({item_lits}))"
157+
92158
# ----- string operations -----
93159

94160
def position(self, needle_lit: str, haystack_expr: str) -> str:

src/flagsmith_sql_flag_engine/translator.py

Lines changed: 20 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
"""Translate `SegmentContext` predicate trees into SQL `WHERE` expressions.
22
33
The translator emits a predicate that goes against an `IDENTITIES` table
4-
holding a `traits` VARIANT column (per `SnowflakeDialect.SCHEMA_DDL`).
5-
Trait-bound conditions become VARIANT path-extractions
6-
(`i.traits:"<key>"`); `PERCENTAGE_SPLIT` and `:semver`-marked comparators
7-
compile to inline pure-SQL using the active `Dialect`.
4+
whose schema is owned by the active `Dialect` (see `dialect.schema_ddl`).
5+
Trait-bound conditions become trait-path lookups via `dialect.trait_path`
6+
— Snowflake's implementation extracts from a `VARIANT` column, but the
7+
translator never assumes that storage shape. `PERCENTAGE_SPLIT` and
8+
`:semver`-marked comparators compile to inline pure-SQL through the
9+
dialect's primitives (hashing, regex, casts, etc.).
810
911
Output shape::
1012
@@ -283,90 +285,6 @@ def _engine_in_values(value: object) -> list[str] | None:
283285
return value.split(",")
284286

285287

286-
def _trait_typed_eq(ctx: TranslateContext, path: str, value: object, negate: bool) -> str:
287-
"""Type-dispatched EQUAL/NOT_EQUAL on a VARIANT trait, mirroring engine
288-
semantics: segment value is cast to the trait's runtime type before
289-
comparison; cast failure → no match for both ops.
290-
291-
EQUAL is emitted as a fast string compare OR'd with type-specific
292-
branches that catch the cases where the variant's `::STRING` form
293-
doesn't equal the literal segment value (booleans stringify lower-
294-
case; floats get expanded to scientific or full-precision forms).
295-
Branches are only emitted when the segment value can be coerced to
296-
that type at translate time, so a string-only segment (`"enterprise"`)
297-
collapses to a single `(path)::STRING = 'enterprise'`.
298-
299-
NOT_EQUAL keeps the `TYPEOF`-dispatched form: it only returns True
300-
when the cast succeeded *and* values differ, which an OR-of-positives
301-
can't express without over-matching."""
302-
d = ctx.dialect
303-
str_value = str(value)
304-
str_lit = string_literal(str_value)
305-
str_path = d.cast_string(path)
306-
307-
# Engine int/float cast: int(v) / float(v); ValueError → no match.
308-
try:
309-
int_lit: str | None = str(int(str_value))
310-
except (ValueError, TypeError):
311-
int_lit = None
312-
try:
313-
float_lit: str | None = repr(float(str_value))
314-
except (ValueError, TypeError):
315-
float_lit = None
316-
317-
if not negate:
318-
# Fast string compare always present — handles VARCHAR traits and
319-
# canonically-stringified INTEGER traits in one cheap branch.
320-
clauses = [f"{str_path} = {str_lit}"]
321-
# Engine bool cast: `lambda v: v not in ("False", "false")`. Compare
322-
# against the variant's lowercase string form — Snowflake's optimiser
323-
# evaluates `(...)::BOOLEAN` eagerly even when the IS_BOOLEAN guard
324-
# would have short-circuited, so we must never feed a non-bool variant
325-
# into a BOOLEAN cast (`100037: Boolean value 'red' is not recognized`).
326-
bool_str_lit = "'false'" if str_value in ("False", "false") else "'true'"
327-
clauses.append(f"(IS_BOOLEAN({path}) AND {str_path} = {bool_str_lit})")
328-
if float_lit is not None:
329-
# Variant float `1.23` stringifies to `'1.230000000000000e+00'`-ish
330-
# in Snowflake — direct string compare misses it, so a typed
331-
# branch is needed. TRY_TO_DOUBLE on the string form sidesteps
332-
# the same eager-eval trap as the bool branch.
333-
clauses.append(
334-
f"((IS_DECIMAL({path}) OR IS_DOUBLE({path}))"
335-
f" AND TRY_TO_DOUBLE({str_path}) = {float_lit})"
336-
)
337-
return "(" + " OR ".join(clauses) + ")"
338-
339-
# NOT_EQUAL: per-type dispatch.
340-
no_match = "FALSE" # engine returns False on cast failure
341-
bool_str_lit = "'false'" if str_value in ("False", "false") else "'true'"
342-
bool_branch = f"{str_path} <> {bool_str_lit}"
343-
int_branch = f"({path})::NUMBER <> {int_lit}" if int_lit is not None else no_match
344-
float_branch = f"({path})::FLOAT <> {float_lit}" if float_lit is not None else no_match
345-
return (
346-
f"((TYPEOF({path}) = 'BOOLEAN' AND {bool_branch})"
347-
f" OR (TYPEOF({path}) = 'INTEGER' AND {int_branch})"
348-
f" OR (TYPEOF({path}) IN ('DECIMAL', 'DOUBLE') AND {float_branch})"
349-
f" OR (TYPEOF({path}) NOT IN ('BOOLEAN', 'INTEGER', 'DECIMAL', 'DOUBLE')"
350-
f" AND {str_path} <> {str_lit}))"
351-
)
352-
353-
354-
def _trait_typed_in(ctx: TranslateContext, path: str, value: object) -> str | None:
355-
"""Type-dispatched IN on a VARIANT trait, mirroring engine semantics:
356-
int trait stringifies and looks up against the parsed string set; string
357-
trait does direct lookup; other types never match.
358-
359-
Collapsed to a single `TYPEOF` gate around one string IN compare —
360-
Snowflake stringifies INTEGER variants without decimals, so the same
361-
`(path)::STRING IN (...)` works for both VARCHAR and INTEGER."""
362-
items = _engine_in_values(value)
363-
if items is None:
364-
return None
365-
item_lits = ",".join(string_literal(v) for v in items)
366-
str_path = ctx.dialect.cast_string(path)
367-
return f"(TYPEOF({path}) IN ('VARCHAR', 'INTEGER') AND {str_path} IN ({item_lits}))"
368-
369-
370288
def _comparison(
371289
ctx: TranslateContext,
372290
op: str,
@@ -376,8 +294,9 @@ def _comparison(
376294
) -> str | None:
377295
"""Emit a SQL fragment comparing `expr` against `value` per `op`.
378296
379-
Used for both trait columns (VARIANT, cast as needed) and JSONPath
380-
references (already-typed columns or string literals). Returns `None`
297+
Used for both trait references (cast via the dialect as needed) and
298+
JSONPath references (already-typed columns or string literals).
299+
Returns `None`
381300
only for genuinely-untranslatable inputs (e.g. RE2-unsafe regex);
382301
inputs the engine evaluates to a deterministic False (missing value,
383302
non-numeric operand on a comparator) compile to `"FALSE"`.
@@ -523,7 +442,9 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
523442
# syntax parsing failed — engine treats the latter as a literal trait
524443
# key. Fall through to the trait branch.
525444

526-
# Trait-bound predicates → VARIANT path-extraction on i.traits:"<key>".
445+
# Trait-bound predicates: the dialect knows how to extract traits from
446+
# whatever shape it stores them in (Snowflake's VARIANT column today,
447+
# Postgres JSONB or a long-form table tomorrow).
527448
path = ctx.trait_path(prop)
528449
if op == "IS_SET":
529450
return f"{path} IS NOT NULL"
@@ -552,16 +473,19 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
552473
f"{_semver_sort_key_expr(ctx, bare_lit)})"
553474
)
554475

555-
# Type-aware comparators on VARIANT traits — mirror flag_engine's
556-
# per-type coercion of the segment value before compare.
476+
# Type-aware comparators on traits — delegate to the dialect, since the
477+
# discriminator (TYPEOF / IS_*), runtime type-coercion casts, and
478+
# short-circuit pitfalls are all engine-specific.
557479
if op in {"EQUAL", "NOT_EQUAL"} and val is not None:
558480
negate = op == "NOT_EQUAL"
559-
return f"({path} IS NOT NULL AND {_trait_typed_eq(ctx, path, val, negate=negate)})"
481+
eq_pred = ctx.dialect.trait_eq(ctx.identities_alias, prop, val, negate=negate)
482+
return f"({path} IS NOT NULL AND {eq_pred})"
560483
if op == "IN":
561-
in_pred = _trait_typed_in(ctx, path, val)
562-
if in_pred is None:
484+
items = _engine_in_values(val)
485+
if items is None:
563486
# Bad IN value (non-string, non-list) — engine returns False.
564487
return "FALSE"
488+
in_pred = ctx.dialect.trait_in(ctx.identities_alias, prop, items)
565489
return f"({path} IS NOT NULL AND {in_pred})"
566490

567491
return _comparison(ctx, op, path, val, is_jsonpath=False)

0 commit comments

Comments
 (0)