mikebridge commented on PR #40221:
URL: https://github.com/apache/superset/pull/40221#issuecomment-4905817027

   **CI unblock for this branch and its stack: #41856.** While validating the 
stacked PRs we found `sl-cache`'s own `unit-tests (current)` is red on the 
workflow's 100% coverage gate for `superset/semantic_layers/` (the branch sits 
at 86.44%), and every stacked PR inherits that red. **#41856** (into this 
branch) closes the gap to 100.00% — 499 tests, mostly table-driven over the 
untested implication/mask/coercion edges — after which this branch's job and 
#41824/#41825/#41826 all go green.
   
   Writing those tests surfaced three things worth this thread's attention:
   
   1. **Real bug, fixed in #41856**: negative leftover masks 
(`NOT_EQUALS`/`NOT_IN`/`NOT_LIKE`) kept NULL rows — pandas `NaN != x` is True, 
SQL three-valued logic excludes it — so cache-served results could contain rows 
the warehouse would have dropped.
   2. **Semantics divergence, flagged**: `_sql_like_to_regex` has no escape 
support — `LIKE '100\%'` wildcards the `%`, unlike e.g. PostgreSQL's default 
backslash escape — so leftover `LIKE` masks can diverge from warehouse behavior 
for escaped patterns. (This is the concrete version of the earlier 
`_sql_like_to_regex` review note.)
   3. **Dead code**: two fallthrough `return False` lines in `_implies_range` 
are unreachable (both same-direction range-op combinations are exhaustively 
enumerated above them) — pragma'd with justification in #41856; deleting them 
is equally fine.
   
   Positive note for confidence in the design: the full pairwise implication 
matrix (every `IS_NULL`/`EQUALS`/`IN`/`NOT_IN`/range cross-combination) passed 
against SQL semantics on the first run — the containment logic's core is sound; 
the gaps were at the mask/NULL boundary.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to