diff --git a/.claude/skills/slayer-models.md b/.claude/skills/slayer-models.md index dc7408fd..8035ed64 100644 --- a/.claude/skills/slayer-models.md +++ b/.claude/skills/slayer-models.md @@ -57,9 +57,10 @@ Models can declare LEFT JOIN relationships to other models: joins: - target_model: customers join_pairs: [["customer_id", "id"]] + cardinality: many_to_one # optional; source→target arity ``` -Enables cross-model measures (`customers.score:avg`), multi-hop dimensions (`customers.regions.name`), and transforms on joined measures (`cumsum(customers.score:avg)`). Auto-ingestion creates one direct join per FK on the source table. Multi-hop paths (e.g. `orders → customers → regions`) are resolved at query time by walking each intermediate model's own joins. Diamond joins (same table via different paths) are supported — each path gets a unique `__`-delimited alias (e.g., `customers__regions` vs `warehouses__regions`). +Enables cross-model measures (`customers.score:avg`), multi-hop dimensions (`customers.regions.name`), and transforms on joined measures (`cumsum(customers.score:avg)`). Auto-ingestion creates one direct join per FK on the source table (composite FKs stay a single join with multiple `join_pairs`). `cardinality` (`one_to_one` / `one_to_many` / `many_to_one` / `many_to_many`, omit when undetermined) is descriptive metadata, orthogonal to the always-LEFT join type; auto-ingestion fills it structurally, and `slayer validate-models --cardinality [--persist-cardinality]` infers it from the data. See [models.md#join-cardinality](../../docs/concepts/models.md#join-cardinality). Multi-hop paths (e.g. `orders → customers → regions`) are resolved at query time by walking each intermediate model's own joins. Diamond joins (same table via different paths) are supported — each path gets a unique `__`-delimited alias (e.g., `customers__regions` vs `warehouses__regions`). **Derived-on-derived chaining.** A `Column.sql` may reference another *derived* column — local same-model or via the join graph (single-dot `B.col` or `__`-delimited `B__C.col` path). Same-model refs can be **bare** (`A.ratio = "bar / foo_normalized"`) or **qualified** (`A.ratio = "A.bar / A.foo_normalized"`) — both inline identically. The engine recursively inlines those references at query time, so you can write `A.ratio = "A.bar / B.foo_normalized"` even when `B.foo_normalized.sql = "foo_raw / 100.0"`. No need to inline derivations at every consumer site. Refs inside a nested scope (sub-query, `UNION` branch, CTE, `VALUES`) are left alone — they belong to the inner rowset. Cycles raise `ColumnCycleError` (a subclass of `ValueError`) at `save_model` time, so a cyclic model never reaches a query. @@ -148,7 +149,7 @@ models = ingest_datasource(datasource=ds, schema="public") ``` Generates: -- One `Column` per non-joined database column (with `type` inferred). PK columns get `primary_key=True`. A column literally named `count` is renamed to `count_col` to avoid clashing with `*:count`. +- One `Column` per non-joined database column (with `type` inferred). PK columns get `primary_key=True`; single-column `UNIQUE` constraints set `unique=True`. A column literally named `count` is renamed to `count_col` to avoid clashing with `*:count`. - `*:count` is always available without an explicit definition; aggregation is picked per query via colon syntax (e.g., `amount:sum`). - **Dynamic joins**: detects FK relationships and emits explicit join metadata (LEFT JOINs built at query time). - FK columns are excluded from joinable models; ID-like columns (`*_id`, `*_key`) are usable as group-by columns only via the `primary_key` flag. diff --git a/DECISIONS.md b/DECISIONS.md index 4b37b87d..391739bd 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -69,9 +69,11 @@ implementation detail. Include issue refs when known. - 2026-08-03 — Optional blocks + Cube JS/FILTER_PARAMS import (DEV-1730 / #270): a Mode-A-only `{? ... ?}` block renders its content parenthesised when every inner `{var}` is supplied, else collapses to the neutral `(1=1)` — the SLayer form of a Cube `FILTER_PARAMS` optional pushdown. Blocks live in the same `substitute_variables` (escape="sql") scanner as `{var}`/`{{`/`}}`, must contain ≥1 var, do not nest, and are rejected in Mode-B. A block-bearing model runs substitution even on a zero-variable call so its blocks collapse (the `_substitute_model_sql_surfaces` fast-path now checks for `{?` too); a block-free, required-only model with zero variables is still left untouched (the documented DEV-1625 raw-brace-literal boundary). `extract_model_variables(model)` derives required (bare, no default) vs optional (in-block or defaulted) from the four Mode-A surfaces — structural, nothing persisted, surfaced additively in the inspect skeleton `Variables:` line. The Cube importer gains a **JavaScript front-end** (esprima ESTree parser, a new core dep) that parses `cube()`/`view()` into the same `CubeCube`/`CubeView` shapes as YAML (dynamic values → report + skip member). FILTER_PARAMS refs are carried JS→converter as structured `CubeFilterParamRef` on the transient `CubeCube` (sentinels in the surface text; no arrow-body re-parse, sidestepping the `{var}`-vs-`{FILTER_PARAMS…}` brace clash); the converter resolves sentinels AFTER `translate_cube_refs` so the introduced `{var}` are never eaten. Requiredness (bare vs block) is decided in the converter alone via `honor_required_meta` (default on; CLI `--ignore-required-meta`) AND the member's `meta.required`; with the flag off a scalar-position arrow collapses to Cube's own `(1=1)::TIMESTAMP` booby-trap, faithfully. Cross-cube refs, unknown members, and generated-name collisions (`d`→`d_from` clashing member `d_from`) drop the cube (`filter_params_unsupported`); each logical variable is reported once (`filter_params_variable`) and stashed in `meta.cube_variables`. `render_probe_text` (blocks→`(1=1)`, bare vars→`0`) is the single import-time validation renderer, matching runtime collapse. - 2026-08-04 — Dialect-aware / complete escaping for Mode-A `{variable}` substitution (DEV-1727), hardening DEV-1625. `substitute_variables(..., escape="sql")` is now **dialect-aware** and **fail-closed**: it gained a required keyword-only `backslash_escapes` signal (`bool | None`, raises if `None` in sql mode) so a caller rendering raw SQL can never silently under-escape. On backslash-escaping dialects (MySQL/ClickHouse/Snowflake/Redshift/BigQuery/Databricks/Spark) it doubles the backslash before escaping the single quote; on standard dialects it keeps the `''` quote-doubling. The double quote is deliberately left untouched — inside a single-quoted literal `\"` is NOT a recognised escape on 6 of the 7 backslash dialects (only MySQL), so escaping it would corrupt the value. The regime is DERIVED from sqlglot's own tokenizer via `SqlDialect.backslash_escapes_strings` (= `"\\" in tokenizer.STRING_ESCAPES`, guarded + 14-dialect pinned) so our escaping can never drift from the parser that reads the substituted SQL. `escape="python"` (Mode-B) additionally encodes the full C0 control range (`\t`/`\n`/`\r` named, rest `\xNN`) so raw newlines/NUL no longer break `ast.parse`. Engine fail-closed: `_substitute_model_sql_surfaces` / `_render_probe_model` require a `dialect`, threaded from the resolved datasource — no bare bool to forget. Assumes MySQL's default `sql_mode` (backslash escapes on); `NO_BACKSLASH_ESCAPES` servers are a sqlglot-layer-wide limitation, documented not fixed. The SQLite backslash end-to-end gap stays a pinned strict-xfail (pre-existing, out of scope). Bound parameters rejected (don't fit substitute-into-raw-SQL). Nested/join/cross-model lineages remain DEV-1678. - 2026-08-04 — Declared list-valued `{variable}` coercion (DEV-1730 follow-up): a scalar supplied for a variable the model declares `list_valued` is wrapped into a one-element list before Mode-A substitution, so an importer-generated `col IN ({var})` renders `IN ('US')` rather than the unquoted `IN (US)`. The generic scalar rule (author writes the quotes, so `{var}` also works in numeric/fragment positions like `amount >= {floor}` and `{d}::TIMESTAMP`) is CORRECT and unchanged — it just presumes an author who can see the SQL position, which a machine-generated fixed template does not have; the caller cannot supply per-element quotes through parentheses the importer wrote. Silent-wrong-answer risk drove the fix over a raise: `region IN (US)` parses as a column reference, so it fails at the database with a confusing message, or resolves against a real column and returns wrong rows. Opt-in is a front-end-NEUTRAL flag: the Cube converter writes `list_valued: ref.kind == "string"` into each `meta.cube_variables` entry (arrow forms splice pre-quoted scalars and stay `False`), and the engine reads only that flag — never Cube's `kind` taxonomy — so a future list-shaped front-end opts in the same way. Coercion lives at the single Mode-A choke point `_substitute_model_sql_surfaces` (execution and the `_render_probe_model` type-probe both route through it, so it cannot be bypassed) via `coerce_declared_list_variables` / `list_valued_variable_names` in `slayer/core/query.py`. Scope is deliberately narrow: only `str`/`int`/`float`/`bool` are wrapped; `list`/`tuple` pass through (the **empty list still raises** — "no filter" belongs to an optional block or a sentinel default); `None`/`dict` are left for `_render_variable_value` to reject with its own naming error; hand-written models declare nothing and are untouched. Follow-on from the same review: `declares_variables(model)` (any non-empty `meta.cube_variables`) now also defeats the DEV-1625 zero-variable fast path, via the shared `_model_needs_substitution_pass` predicate used by both `_substitute_model_sql_surfaces` and `_render_probe_model`. This closes the fast-path hole for a GENERATED model whose pushdowns are all required (no `{? ?}` block to force the pass): such a model used to emit a bare `{var}` into the SQL on a zero-variable call instead of raising the documented missing-variable error. The hole stays open — deliberately — for hand-written models, which declare nothing and keep the raw-brace-literal protection (`'{1,2,3}'`). The `list_valued` flag is matched with `is True`, not truthiness, since `meta` is user-extensible and a stray `1` or the string `"false"` must not switch substitution semantics. The bag is also SELF-IDENTIFYING — an entry counts only with a string `member` (the shape every importer writes) — so a hand-written `meta` that reuses the `cube_variables` key is not mistaken for generated SQL and silently stripped of its brace-literal protection. +- 2026-08-04 — Join arity is first-class but representation-only (DEV-1688): `ModelJoin.cardinality` (`one_to_one`/`one_to_many`/`many_to_one`/`many_to_many`, read source→target) plus `Column.unique` are additive/optional, so no `SlayerModel` version bump. Cardinality is deliberately ORTHOGONAL to `join_type` — FK-derived joins stay LEFT, never auto-INNER, because FK enforcement is DB-specific and nullable FKs are common, and an INNER hop would break the "adding a measure/join never changes result cardinality" invariant. It is metadata only: not threaded into the SQL generator, `resolved_joins`, or `CrossModelMeasure` (query-time fan-out use is a follow-up). Inference follows an asymmetric-evidence model — a declared PK/unique constraint guarantees uniqueness, a full scan can *disprove* it with one duplicate but can only *suggest* it — so structural inference at ingest leaves cardinality unset unless the target key is verified unique, and data profiling (`slayer validate-models --cardinality`, report-only unless `--persist-cardinality`) marks a mismatch `contradicts_hard` only when observed duplicates disprove a uniqueness the stored value asserted, `refines` otherwise. An empty key population is NO evidence (not weak evidence of uniqueness) and reports a distinct `no_evidence` verdict, detecting and persisting nothing. Profiling is folded into `validate-models` rather than a `slayer joins` group of its own: one command answers "is my model still true of the database", with the expensive full-scan half opt-in behind a flag; the engine methods stay separate so MCP / REST `validate_models` keep their metadata-only cost. A per-join scan failure is contained as a `scan_failed` finding (one unreadable table must not cost the whole report) EXCEPT for a fail-closed `ForcedFilterError`, which propagates — downgrading a policy error to a report line would hand back unscoped statistics. The CLI also enumerates datasources itself on an unscoped run (concurrently, but attributed per datasource), since `validate_models(None)` suppresses per-datasource failures and a validation command must not report a clean bill for a datasource it never reached. Exit-code rule: 0 for any report ABOUT the data (`contradicts_hard` included — the command worked, the answer is just unwelcome), 1 whenever the command could not do its job (unknown scope, datasource failure, profiling failure, any `scan_failed`, residual drift after `--force-clean`). Containment is not silence: a contained scan failure still prints in the report AND exits 1. Diagnostics go to stderr so stdout stays parseable under `--format json`. A side counts as unique iff some unique key-set is a non-empty SUBSET of the join key — so a member of a composite PK, an expression-index member, and a partial (predicate-filtered) unique index all fail to establish solo uniqueness. Composite FKs now ingest as one grouped join instead of being shredded per column, and cross-schema FKs are skipped rather than bound to a same-named local table. Profiling scans route through the RLS session policy like every other execution path. Cardinality/`unique` are excluded from the embedded search corpus so no re-embedding fires. - 2026-08-05 — Ingestion sees views, survives unmodellable names, and stops being silent (DEV-1741). **Views**: `list_ingestable_objects` replaces the bare `get_table_names()` at every introspection site, adding `get_view_names` + `get_materialized_view_names` behind a `NotImplementedError`/`Exception` guard (the base `Inspector` RAISES for matviews on unsupporting dialects rather than returning `[]`), de-duplicated first-classification-wins because some dialects return views from `get_table_names()`, in a deterministic tables→views→matviews order that the name-collision policy depends on. Ingested by default — dbt materializes staging models as views, so opt-in would have left the reported failure in place for a fresh install — with `--no-views` on both `slayer ingest` and `datasources create --ingest`. The drift side (`_live_schema_for_datasource`) and the MCP listing (`_fetch_tables`) take **no flag and are unconditional**: that map is only ever a lookup target (`validate_datasource` iterates the *persisted* models, `available_in_ds` derives from them), so views there cannot manufacture a model or a drift entry — what they fix is a pre-existing **data-loss** bug where a hand-authored model whose `sql_table` named a view resolved to `live_table=None` → `WholeModelDelete` → deleted by `validate-models --force-clean`. Gating that on `--no-views` would re-arm it for exactly the users who opted out. **Names**: model names can't contain `__` (six modules — generator/enrichment/column_expansion/column_dependency/schema_drift/osi — `split("__")` an alias back into a join path, so a model named `a__b` is read as the alias for `a→b` and yields a silently wrong query, not a crash), but `sql_table` can, so a dlt child table `reports__patient__drug` is modelled as `reports_patient_drug` with `sql_table` verbatim. Sanitizer is `re.sub(r"_{2,}", "_")`, NOT `replace("__","_")` — `str.replace` is non-overlapping, so `a___b`→`a__b` would still fail validation. Collisions reserve every unsanitized name first (a real `a_b` always beats a sanitized `a__b`, order-independently) and **skip** rather than suffix, since suffixes shift as the object set changes and would orphan models and churn drift. A per-object `try/except` backstops everything else (`.`/`:`/`/`/`\` names, bad column names, per-object introspection failures); the FK-collection loop and `_get_fk_relationships` are guarded too because they run BEFORE that isolation and would otherwise still kill the run. **Reporting**: skips travel in a new `skipped` list, deliberately NOT folded into `errors` — `slayer ingest` exits 1 on either (we declined a perfectly valid object; `--exclude` is the documented remedy) but `POST /ingest` keeps 422 for `errors` only, because a permanent 422 aimed at a machine that can't act on the hint buries a successful partial ingest behind an error status. An empty scan prints the available schemas and exits 1 (the reporter listed the missing exit code as part of the defect), gated on `objects` not `additions` so a healthy no-op re-ingest stays quiet; `datasources create --ingest` on an empty DB still exits 0, since creating the datasource is that command's job and it succeeded. `in_scope_table_names` switched from model names to `_bare_table_name(sql_table)` — it is compared against table names in `_scoped_models_for_validation`, so the old keying silently dropped from validation scope any model whose name differs from its table (every sanitized model, and already every dbt/OSI hidden model passing `model_name=`). **`source_kind`** (`table`/`view`/`materialized_view`/`None`=unknown) persists on `SlayerModel`, v7→v8 with a no-op migration (mandatory: `migrate()` raises `RuntimeError` on an unregistered step); `None` for pre-v8, hand-authored and sql/query-backed models is the honest value, not a guess. It is a deliberate **exception to the additive-merge contract** — refreshed, not preserved — because it describes the live object rather than user intent, and the transition it exists to capture (dbt `+materialized: table`) usually changes no columns at all; the refresh therefore has to be made in three places (the early return, the `model_copy(update=...)`, and the save gate in `_process_one_table`), since editing only the update dict computes a corrected model and throws it away. A `None` from a non-classifying path never erases a known value. Docs-only fix for the advertised `motley-slayer[duckdb]` extra, which does not exist: `duckdb`/`duckdb-engine` are unconditional core deps (the Postgres facade imports duckdb at top level on every CLI invocation), and adding them under `[tool.poetry.extras]` would *gate* rather than alias them, breaking bare `pip install motley-slayer` → `datasources create demo`. - 2026-08-06 — Dialect-aware identifier-length fitting (DEV-1756): every dialect declares a conservative universal budget as `SqlDialect.max_identifier_bytes` (postgres 63, mysql 64, redshift 127, oracle/tsql 128, snowflake 255, duckdb 256, bigquery 300, `None` = unbounded for sqlite/clickhouse/trino/presto/databricks/spark), and an over-limit identifier is shortened at emission to `__` via `slayer/sql/dialects/_identifier_fit.py`. Postgres is the binding case and the reason this is a correctness fix rather than cosmetics: it truncates over-length identifiers **silently** (a NOTICE, never an error), so two 3-hop aliases sharing a 63-byte prefix either blow up as `AmbiguousColumnError` under the DEV-1444 outer wrap or — with no sibling to collide with — quietly return the column under a name the engine never looks up. Shorten **only when over the limit**, never uniformly: `decode_result_keys` restores the canonical dotted alias, so the dialect-dependence the issue worried about is invisible to consumers and the 99% case keeps byte-identical SQL (pinned by pre-change goldens, not merely by idempotence). Both ends of the alias are kept because the reported colliding pair differs ONLY in its final segment — a head-only truncation would render the two indistinguishable in `dry_run` output. The digest is sha256 of the FULL original, which is what makes `fit_identifier` a pure function of the name and lets the read side rebuild the emitted→canonical map by re-running it, with no map threaded through generation (the alternative the issue sketched). Write side is an EXACT-match replacement over the query's own alias set (`all_projection_aliases`, unfiltered — hidden ORDER-BY hoists and `_inner_*`/`_ft*`/`_ts*` entries are projected in the inner SELECT and truncate identically), never a length regex over arbitrary SQL, so a long quoted-looking span inside a string literal can never be corrupted; substitution is two-phase (canonical→sentinel→final) as defence-in-depth, though today's key set (over-limit) and value set (within-limit) are provably disjoint. BigQuery/T-SQL compose by running their existing dot-mangle regex AFTER the base length pass: an under-limit alias makes the length pass a genuine no-op so their output is unchanged, and an over-limit one arrives still-dotted and gets mangled by the same regex — no double-encoding — with the budget sized against `encode_alias` so the post-mangle form still fits. Scope is the three OUTPUT-name surfaces: projection aliases, CTE names (`_cte_name_from_alias` fits the whole result, prefix included, allocated through a per-statement collision-checked `SQLGenerator._cte_name`), and `_query_as_model` virtual-model shorts. Join-path TABLE aliases are deferred to DEV-1743, whose plan already owns the `__` path-alias allocator; their failure mode (silent wrong joins) is worse but the fix requires decoupling `EnrichedDimension.model_name` from the emitted qualifier across ~8 sites that split it back on `__`. Collisions raise `IdentifierCollisionError` rather than emitting ambiguous SQL — the check covers identity entries too, since an already-short alias equal to another's fitted form is a duplicate no hash width can prevent. Fixed alongside, in the same code path: `_query_as_model` no longer decides its short alias's quoting with a hand-written predicate. The contract is AGREEMENT between the emitted `AS ` and the downstream `Column(sql=short)` reference — they must resolve to the same column — so the short is now run through the same two mechanisms the reference side uses: `SQLGenerator._maybe_quote_ident` (DEV-1645: quote iff the name contains an uppercase letter) followed by `Identifier.sql(dialect=...)`, which lets sqlglot add quotes for words reserved IN THE TARGET DIALECT and for names that are not safe bare identifiers. Emitted bare, a mixed-case short was case-folded by Postgres while the outer stage referenced it quoted, making any query-backed model with a mixed-case join path unqueryable (`UndefinedColumnError`, reproduced on a live server). Two wrong answers were tried and rejected on the way: quoting UNCONDITIONALLY defines a case-sensitive `"status"` on upper-folding backends (Snowflake, Oracle) while the bare reference still resolves as `STATUS`, trading a Postgres bug for a Snowflake one; and quoting on `SLAYER_RESERVED_KEYWORDS` alone misses words reserved only NATIVELY (`index`, `int`, `rows` on MySQL; `rows` on BigQuery), where the reference side quotes but a bare `AS index` is a syntax error — `install_reserved_keywords` unions our set INTO each generator's, so only the generator knows the full set. Shape matters too: `Column.name` forbids only `.` and `:`, so `1abc` or `foo bar` needs quoting on form alone. Both axes must therefore be dialect-driven, and the rule is pinned by a 12-dialect × 7-name test asserting the two spellings are byte-identical. Because an all-lowercase short stays bare and still shares a case-folded namespace, the short-uniqueness check remains case-insensitive. - 2026-08-06 — `mcp` capped at `>=1.0,<2` (DEV-1757). mcp 2.0.0 renamed `mcp.server.fastmcp` → `mcp.server.mcpserver` (`FastMCP` → `MCPServer`), so the unbounded `mcp = ">=1.0"` let every lockfile-free install (`pip install motley-slayer`, `uv tool install`) resolve a major `slayer/mcp/server.py` cannot import — a broken MCP server, SLayer's primary agent-facing interface, on every fresh install. `poetry.lock` pinned a 1.x, so CI was green throughout and only users saw it; the guard added here (`tests/test_mcp_dependency_pin.py`) is therefore **declaration-level** — it parses the pyproject constraint and asserts 2.0.0 falls outside it, since no in-repo test can execute an mcp major the environment does not have. Two consequences accepted deliberately: users needing mcp≥2 for another package in the same environment now hit a resolver conflict instead of a runtime crash, and other core deps were left unbounded — capping the rest was considered and rejected (caps rot and produce unresolvable trees downstream), which is the scope of this change and NOT a standing no-caps policy. The import failure now diagnoses itself: absent package, wrong major, and a 1.x that failed to import for some other reason are three distinct messages, all offering `pip install 'mcp>=1.0,<2'` first and "upgrade SLayer" second — the old "Reinstall SLayer: pip install motley-slayer" text told users to do the one thing that reproduced the failure, and leading with the upgrade hint would recreate that for anyone already on the latest release. Separately, `serverInfo.version` now reports SLayer's own version: FastMCP 1.x exposes no `version` kwarg and never forwards one to the lowlevel `Server`, which falls back to `pkg_version("mcp")`, so SLayer was announcing the SDK's version as its own. The stamp writes the private `_mcp_server.version` (the only route in 1.x; the file already sets `_slayer_engine` on the same object per DEV-1656) and tolerates both a missing attribute and a read-only one, so a future SDK cannot abort server construction over a cosmetic field. Migrating to the 2.x `MCPServer` API — which would retire that private write via its public `version=` kwarg, and also pulls in `Context` injection, worker-thread sync handlers, snake_case `mcp.types`, and an httpx→httpx2 / pydantic≥2.12 / opentelemetry dependency shift — is deferred, as is removing the deprecated `inspect_model` tool. - 2026-08-06 — Recognised ELT/migration housekeeping tables ingest hidden (DEV-1759). An unfiltered ingest modelled `_dlt_loads`/`_dlt_pipeline_state`/`_dlt_version` as first-class models; the model list is the menu handed to an agent over MCP, so junk entries burn tokens every session, invite an agent to aggregate or join them (agents lack the human instinct that a `_`-prefixed table is off-limits), and dump kilobytes of serialized state into context on one exploratory query. **Hidden, not skipped** — the model exists, stays queryable by name and remains a valid join target, so DEV-1741's no-silent-omissions principle holds and the deliberate use the issue names (`_dlt_loads` answering "when did this last load?") keeps working; all it loses is presence in `models_summary`, `models list`, the REST listing, semantic search, and the BI catalogs, each of which already gated on `model.hidden`. Rules live in a pure, DB-free `slayer/engine/internal_tables.py`; matching is case-insensitive (Liquibase upper-cases, EF Core writes `__EFMigrationsHistory`, Sequelize `SequelizeMeta`) and runs on the **live object name**, never the model name, since `_assign_model_names` collapses `__` runs and matching a derived string is matching something the database never had. PREFIX rules are admitted only where the namespace is reserved by contract in every warehouse the vendor loads into, so the rule needs no dialect: `_dlt_` and `_airbyte_` (which is also how `_airbyte_raw_*`'s arbitrary stream suffix is covered); everything else is an exact name. No `_fivetran_`/`_sdc_` prefix rule: both vendors' real surface is *columns* on real tables (`_fivetran_synced`, `_sdc_batched_at`), so a table-level prefix would match nothing and imply coverage we do not have — Fivetran's only destination-schema tables are the two audit ones, listed exactly. **No `sqlite_` prefix rule either**, for the same reason plus a worse one, and this took a second pass to see: SQLite genuinely reserves the namespace (`CREATE TABLE sqlite_foo` is a hard error), but the objects it reserves it FOR never reach the scan, because SQLAlchemy's SQLite inspector defaults to `sqlite_include_internal=False` and filters `sqlite_sequence`/`sqlite_stat1`…`sqlite_stat4` out of `get_table_names()`. So the rule was unreachable on the one dialect that justified it, and its only reachable effect was on a NON-SQLite datasource — where nothing reserves the prefix and `sqlite_backup` is an ordinary table — i.e. hiding real user data. Dialect-scoping it was considered and rejected: it would have left machinery gating a rule that then fires nowhere. Both halves of that argument are pinned by tests (the inspector really does drop a live `sqlite_sequence`; SQLite really does reject the `CREATE TABLE`), since deleting a rule on the strength of upstream behaviour is only safe while that behaviour holds. PostGIS and `pg_stat_statements` are deliberately excluded: bookkeeping, but not engine-reserved namespaces, and admitting them opens "which extensions count?" with no principled stopping point. One accepted risk remains: `schema_version` (legacy Flyway ≤4) is the most collision-prone exact entry — recoverable precisely because hiding is not omitting. **Creation-only**: `hidden`/`meta` are absent from `_additive_merge_existing`'s update dict, so an un-hide sticks forever and a pre-existing visible model is never retro-hidden; no migration heals old stores, because a "was this ever edited?" heuristic cannot tell a deliberate un-hide from a pristine model and would re-hide it on every run — the exact fight hidden-not-skipped exists to avoid. Flag is `--surface-internals`, not the issue's `--include-internals`, which would read as "add internals to the `--include` list" next to two flags that genuinely control inclusion; `--include _dlt_loads` therefore still hides it, keeping CLI and REST (which has no flags) identical. **Reporting is split by path, and this is the subtle part**: `IngestionScanReport.hidden_internals` is what the scan CONSTRUCTED (correct for `datasources create --ingest`, which never reads storage), while `IdempotentIngestResult.hidden_internals` is EFFECTIVE post-merge state, re-derived by re-reading each matched model and keeping only those actually hidden — looked up by `model_name`, not `table_name`, or every `__`-sanitized model silently vanishes from the report. Forwarding the scan's verdict lies in both directions: it reports a user-un-hidden model as hidden, and it prints nothing for a still-hidden model on the very run where `--surface-internals` was passed to see it. Candidates are re-derived from `scan.models` rather than reused from `scan.hidden_internals` for that second reason (the latter is empty under the flag by construction) and, being keyed on successfully-built models, exclude skipped objects for free. The `HiddenInternal` entry is appended only AFTER `_build_one_model` returns, so a name collision or a construction failure lands in `skipped` alone — never two contradictory verdicts on one table. Exit code is unchanged (hiding is the intended outcome; a skip is a capability failure with `--exclude` as its remedy) and REST never 422s on it, matching the `skipped` contract. `datasources create --ingest` switched from `ingest_datasource` to `ingest_datasource_report` so it stops being silent about both internals and skips; that forced `_print_ingest_drift_and_errors` to read every field through `getattr`, since an `IngestionScanReport` has no `to_delete` and no `errors` at all. **MCP was the last silent surface, and the one that mattered most**: `ingest_datasource_models` renders through its own `_render_ingest_result` rather than `_print_ingest_drift_and_errors`, and that renderer read only `additions`/`to_delete`/`errors` — so the agent-facing tool this feature exists for was the one place that never said what it hid (nor, since DEV-1741, what it skipped). Both sections were added there in the CLI's order. The subtle half is the EARLY RETURN, not the sections: a steady-state re-ingest produces no additions, no drift and no errors, so the "already in sync" branch would have swallowed both on every run after the first — the guard therefore has to test `skipped` and `hidden_internals` too, while still leaving the empty-schema probe reachable for a genuinely empty schema. The hint is `edit_model(name, hidden=false)` rather than `--surface-internals`, which is a CLI flag this tool does not accept; likewise the skip line names no `--exclude`. `surface_internals` is deliberately NOT exposed as an MCP tool argument: handing the agent a knob to un-hide the junk the feature exists to hide it from inverts the point, and `edit_model` already covers the deliberate single-model case. That un-hide hint is DATASOURCE-QUALIFIED on every surface (shared `_unhide_hint`), because internals are the one model class that collides across datasources *by construction* rather than by accident — `_dlt_loads` exists verbatim in every dlt-loaded database, so a bare `edit_model("_dlt_loads", hidden=false)` resolves by priority list and either raises `AmbiguousModelError` or silently un-hides the other pipeline's model, precisely in the multi-pipeline setup this feature targets. The parameter stays optional because neither result shape carries the datasource — it has to come from the caller, and all four in-tree ones pass it. Column-level internals (`_dlt_id`, `_dlt_load_id`, `_fivetran_synced`, `_sdc_*`) are deferred — `_fivetran_deleted` is a soft-delete flag an agent must SEE or it silently returns deleted rows, which needs its own design pass rather than a rider. Glob `--exclude`, internal schemas (`airbyte_internal`, `fivetran_metadata`), and Airbyte V2's `airbyte_internal._raw__stream_` naming are out of scope. - 2026-08-12 — Formula measure referencing a sibling saved measure now emits valid SQL regardless of measure order (DEV-1779). A saved formula (`habit_score = order_count / unique_customers`) inline-expands at parse time to leaf colon refs (`id:count / customer:count_distinct`), so when the formula measure is enriched BEFORE a referenced sibling, its expression SQL freezes the sibling's canonical alias (`orders.id_count`); the later direct selection of that sibling renames the base-CTE column to the declared name (`orders.order_count`) and the frozen reference dangled — invalid SQL on Postgres, silently-NULL on SQLite (double-quote-as-string-literal). The DEV-1444 provenance-merge only reconciled the forward order (sibling declared first). Fix makes the rename atomic via one `_repoint_alias(prev, new)` helper called at BOTH rename sites (local-agg and cross-model-intercept): it sweeps every `known_aliases` value, the `measure_canonical_key_to_alias` provenance index, and — the new part — the already-frozen carriers `EnrichedExpression.sql` (exact quoted-token replace; the closing quote makes `"orders.id_count"` never match `"orders.id_count_2"`) and `EnrichedTransform.measure_alias` (so `cumsum(order_count)` and `change_pct` desugaring follow the rename too). Quoted-token string replacement is SQL-token-blind but safe here because arithmetic expression SQL is compiler-produced and never embeds a single-quoted literal containing a double-quoted alias — same invariant `_resolve_sql` already relies on. Defense-in-depth: the SQL generator's CTE-layering loop previously emitted an unresolved expression (and silently DROPPED an unresolved self-join `time_shift`) when it stalled, so a regression of this class reached the DB as broken SQL; it now raises a precise `ValueError` naming the computed column / transform and the missing alias for expressions AND all transform types. `_deps_available` gates in-loop addition, so anything still pending is genuinely unresolved — no false-positive raise. - 2026-08-12 — Dotted dimension join-path binding (DEV-1780): a dotted dimension/time-dimension path resolves only when every hop is a direct join. Previously a hop that was not a direct join fell through leniently — the enriched dim kept its `A__B` alias in SELECT/GROUP BY but `_resolve_joins` emitted no join, shipping invalid SQL (unbound table alias). Filters and cross-model measures already rejected such paths; only dimensions/time-dimensions had the hole (the shared `_resolve_dotted_dim_with_stage_fallback` lenient branch). Fix is an engine routing pre-pass (`SlayerQueryEngine._route_dotted_dimension_refs`, run in `_enrich` before `enrich_query`, gated on `enforce_join_binding and source_model_origin is None`): it normalizes root-prefixes via `strip_source_model_prefix`, then for each dotted ref tries the explicit direct-join walk and, on `_NoJoinError`, routes via a datasource-scoped `JoinGraph`. A SHORT FORM (one model segment, e.g. `Consumer.name`) with exactly ONE route to the target auto-resolves — the ref is rewritten to the full routed path, so the result key is the full path (`root.Subscription.Customer.Consumer.name`), consistent with "joined dims keep the full path". Ambiguous (≥2 routes), unreachable (0), and explicit multi-hop chains with a broken hop are REJECTED with `UnresolvableDimensionJoinError(SlayerError, ValueError)` (mirrors the DEV-1645 `UnresolvableOrderColumnError` reject-don't-emit-invalid-SQL doctrine); the message suggests the short form when the target is uniquely reachable, else the shortest deterministic full path (`JoinGraph.shortest_path`), else nothing. `JoinGraph.count_simple_paths(root, target, cap=2)` classifies routes — it counts ALL simple paths (a 2-hop + 3-hop route is genuinely ambiguous; auto-picking the shorter would silently change join semantics), reverse-reachability-pruned and cycle-guarded. The rewrite map is also applied to matching `OrderItem.column` refs and `main_time_dimension` so dependent references stay consistent. Deliberate limits (conservative, prefer reject over a wrong route): routing runs only within a single datasource (`model.data_source` truthy; the graph is datasource-scoped) and is deferred when named-query stages are in scope (their virtual models aren't in the stored graph) — those refs fall through to the guard. A post-`_resolve_joins` safety-net guard in `enrich_query` (same gate) raises `UnresolvableDimensionJoinError` for any dim/time-dim whose alias is absent from `resolved_joins`, guaranteeing the invariant even for direct `enrich_query` callers; the re-rooted cross-model CTE enrichment passes `enforce_join_binding=False` (it legitimately carries source-local shared dims like `orders.status` that never bind to a base-table join). Out of scope: the multi-stage lenient cross-stage fall-through (`test_unresolvable_dotted_ref_falls_through`, where distinguishing a genuine error from a re-rooting artifact is unsolved) and leaf-column-missing-on-a-valid-path (the alias IS bound there — a different failure class). +- 2026-08-16 — FK-derived joins name the MODEL, not the live object (DEV-1688 / DEV-1741 / #279). Model names strip `__` (reserved for join paths), so an FK to `reports__patient__drug` used to persist a join targeting a model that cannot exist; `_generate_joins` now takes the live→model map, and a target whose object was skipped on a sanitization collision drops its join rather than dangling. Stores written before the fix self-heal on the normal re-ingest path rather than via a schema migration — no version bump, and the repair demands the sanitized target AND identical `join_pairs` to match a freshly-generated join, so it can only rename the join the bug produced (name-only matching would collapse `a__b` and `a___b` onto one target and trip the duplicate-target guard, turning a merely-dangling store into a failed re-ingest). A store that never re-ingests keeps a join that was already broken. diff --git a/docs/concepts/ingestion.md b/docs/concepts/ingestion.md index 09cff708..89fa505b 100644 --- a/docs/concepts/ingestion.md +++ b/docs/concepts/ingestion.md @@ -42,6 +42,7 @@ Tables with no FK references use their plain table name with no joins. SLayer introspects each table's column types and generates a model: - **One `Column`** per non-joined column on the source table — name, `type` inferred from the database (`string` / `number` / `boolean` / `time` / `date`), `primary_key=True` for PKs. Whether each column is used as a group-by dimension or as an aggregation source is decided per query. +- **`unique=True`** for columns that alone form a `UNIQUE` constraint or unique index. PK columns are not stamped redundantly — `primary_key` already implies uniqueness. Composite uniqueness is evaluated per key-set during join-cardinality inference rather than being flattened onto individual columns. - **A column literally named `count`** is renamed to `count_col` to avoid clashing with the always-available `*:count`. - **No auto-generated `measures`** — `SlayerModel.measures` is the named-formula library and stays empty after ingestion. You can add named formulas later via the API/MCP if you want bare-name shortcuts (`{"formula": "aov"}`). - **`*:count`** is always available without any model definition. @@ -49,7 +50,9 @@ SLayer introspects each table's column types and generates a model: FK columns from referenced tables are excluded from the source model to avoid redundancy — they're reachable via the join graph as `customers.id` etc. -All models use `sql_table` (the source table) plus `joins` (direct FK joins only, storing source/target column pairs). Multi-hop JOINs are resolved dynamically at query time by walking the join graph. +All models use `sql_table` (the source table) plus `joins` (direct FK joins only, storing source/target column pairs). Multi-hop JOINs are resolved dynamically at query time by walking the join graph. A **composite** foreign key becomes a single join carrying all of its column pairs, not one join per column. + +Each FK join also gets a structural [`cardinality`](models.md#join-cardinality) guess from the key constraints alone (no data is read): `many_to_one` by default, upgrading to `one_to_one` when the source key is itself unique. A side counts as unique only when some PK/unique key-set is a subset of the join key — if `(a)` is unique then `(a, b)` is too, but a constraint on `(a, b)` does not make `(a)` unique. When the target key cannot be *verified* unique from its constraints, cardinality is left unset rather than guessed. To infer it from the data instead, run `slayer validate-models --cardinality`. ### SQLite affinity probing @@ -273,7 +276,7 @@ Ingest-on-startup: N/M datasources ingested (K failed: name1, name2) `slayer ingest` (and the equivalent MCP / REST entry points) is idempotent by default — re-runs are safe. For each in-scope live table: - **No persisted model with that name** → ingest from scratch via the path above. -- **Existing `sql_table`-mode model** → append new columns and joins from the live schema. Existing columns and joins are **never** mutated — `description`, `label`, `format`, `meta`, and `allowed_aggregations` are preserved verbatim. +- **Existing `sql_table`-mode model** → append new columns and joins from the live schema. Existing user metadata is **never** overwritten — `description`, `label`, `format`, `meta`, and `allowed_aggregations` are preserved verbatim. The only in-place updates are strictly additive gap-fills: a join's `cardinality` is set only when it is currently unset (a value you chose is never replaced), and a column's `unique` is only ever turned on, never off. Filling either one is enough to trigger a save, so a re-ingest that adds no columns or joins still persists newly-discovered constraint metadata. - **Existing `sql`-mode or query-backed model with the matching name** → skipped silently; those are user-authored. With the default YAML storage, two live tables whose quoted names differ only by letter case (`"Orders"` vs `orders`) cannot both be persisted — model names collide as filenames on macOS / Windows, so the save is rejected (`IdCollisionError`). The first table wins; the second surfaces as a per-model entry in `IdempotentIngestResult.errors` (or a per-model message on the CLI / MCP paths) without aborting the rest of the ingest. SQLite storage persists both. diff --git a/docs/concepts/models.md b/docs/concepts/models.md index 5252d143..ee495509 100644 --- a/docs/concepts/models.md +++ b/docs/concepts/models.md @@ -77,6 +77,7 @@ A column is the unit of structure on the model. The same column entry can serve | `sql` | string | No | (bare column name) | SQL expression — defaults to the column's name | | `type` | string | No | `string` | `string`, `number`, `boolean`, `time`, `date` | | `primary_key` | bool | No | `false` | Restricts aggregation to `count` / `count_distinct` | +| `unique` | bool | No | `false` | Single-column uniqueness (non-PK). `primary_key` implies unique. Auto-set from `UNIQUE` constraints / unique indexes; used to infer one-to-one joins | | `hidden` | bool | No | `false` | Hide from listings | | `format` | dict | No | — | `NumberFormat` used by response metadata | | `allowed_aggregations` | list[str] | No | — | Whitelist (must be a subset of the type-default eligibility set, or a custom aggregation defined on this model) | @@ -301,6 +302,30 @@ joins: Joins enable **cross-model measures** — querying a measure from a joined model alongside the main model's data. See [Cross-Model Measures](queries.md#cross-model-measures). During [auto-ingestion](ingestion.md), joins are generated automatically from foreign-key relationships; multi-hop paths are resolved at query time by walking each intermediate model's own joins. +### Join cardinality + +A join optionally records its **arity**, read source→target: + +```yaml +joins: + - target_model: customers + join_pairs: [["customer_id", "id"]] + cardinality: many_to_one # many orders → one customer +``` + +`cardinality` is one of `one_to_one`, `one_to_many`, `many_to_one`, `many_to_many` (omit it when undetermined). It is **descriptive metadata, orthogonal to the join type** — joins stay LEFT regardless — and is representational today (query results are unaffected). + +Auto-ingestion fills it structurally from key constraints: an FK join defaults to `many_to_one`, upgrading to `one_to_one` when the source key is itself unique. To infer it from the actual data instead, run: + +```bash +slayer validate-models --datasource mydb --cardinality # report only +slayer validate-models --datasource mydb --cardinality --persist-cardinality # write it back +``` + +Detection full-scans each side of the join and reports the observed arity, a `verdict` (whether it confirms, refines, or hard-contradicts the stored value), and any column declared `unique` that the data shows has duplicates. It is a strong guess, not a guarantee — a duplicate disproves uniqueness with certainty, but the absence of duplicates only suggests it. + +A side with no non-null key rows reports `no_evidence` and detects nothing: an empty scan would trivially look unique, and that is not weak evidence — it is none. Re-run once the table has data. A join whose scan fails outright reports `scan_failed` and does not stop the rest of the report. Full verdict table: [CLI reference](../reference/cli.md#slayer-validate-models). + ### Path-based table aliases Joined tables use `__`-delimited path aliases in generated SQL so **diamond joins** stay unambiguous — when the same table is reachable via multiple paths. For example, if `orders` joins both `customers` and `warehouses`, each referencing `regions`: diff --git a/docs/concepts/schema-drift.md b/docs/concepts/schema-drift.md index 04f722d7..5d3920ac 100644 --- a/docs/concepts/schema-drift.md +++ b/docs/concepts/schema-drift.md @@ -103,11 +103,17 @@ be no-ops once the model is gone). * **REST.** `POST /validate-models` — read-only. Query-time failures attributed to drift surface as **HTTP 422** with body `{error: "schema_drift", models, to_delete, original}`. -* **CLI.** `slayer validate-models [--datasource X] [--force-clean] - [--yes]`. Without `--force-clean`, prints the diff and exits 0. - `--force-clean` prompts (or skips with `--yes`), applies via - `apply_drift_deletes`, and exits non-zero on per-entry errors or - non-empty residual drift. +* **CLI.** `slayer validate-models [--datasource X] [--model M] + [--format text|json] [--force-clean] [--yes]`. Without `--force-clean`, + prints the diff and exits 0. `--force-clean` prompts (or skips with + `--yes`), applies via `apply_drift_deletes`, and exits non-zero on + per-entry errors or non-empty residual drift. `--model` scopes both the + report and the applied deletes. Unlike the engine method, an unscoped CLI + run validates each datasource explicitly and exits 1 if any of them fails, + rather than dropping it from the result. `--cardinality` adds an opt-in + [join-arity profiling](models.md#join-cardinality) pass, which runs after + any `--force-clean` apply. See the + [CLI reference](../reference/cli.md#slayer-validate-models). `--force-clean` is intentionally CLI-only — destructive auto-application must be opt-in at the human-typed layer. diff --git a/docs/dbt/dbt_import.md b/docs/dbt/dbt_import.md index cba512e3..da677c45 100644 --- a/docs/dbt/dbt_import.md +++ b/docs/dbt/dbt_import.md @@ -48,8 +48,11 @@ The converter builds an entity registry by scanning all models, then resolves fo joins: - target_model: customers join_pairs: [["customer_id", "id"]] + cardinality: many_to_one ``` +Each generated join carries a [`cardinality`](../concepts/models.md#join-cardinality) read source→target: a foreign→primary entity reference is `many_to_one`, while a **peer** join (two models sharing the same primary/unique entity) is `one_to_one`. The reverse INNER edge the converter mirrors onto the target model carries the inverted arity. + ### Measures — Column + ModelMeasure Split dbt bakes aggregation into each measure (`agg: sum`). SLayer separates them — a row-level expression lives on a `Column`, and the aggregation is named on a `ModelMeasure` formula. diff --git a/docs/interfaces/cli.md b/docs/interfaces/cli.md index 6d0b9d70..cf333ece 100644 --- a/docs/interfaces/cli.md +++ b/docs/interfaces/cli.md @@ -110,6 +110,30 @@ slayer ingest --datasource my_postgres --exclude migrations,django_session | `--exclude` | No | Comma-separated tables to exclude | | `--storage` | No | Storage path | +### `slayer validate-models` + +Diff persisted models against the live database schemas (read-only), and optionally profile each join's arity from the data. See [Schema Drift](../concepts/schema-drift.md) and [Join cardinality](../concepts/models.md#join-cardinality). + +```bash +slayer validate-models +slayer validate-models --datasource jaffle_shop +slayer validate-models --model orders --format json +slayer validate-models --cardinality --persist-cardinality +slayer validate-models --force-clean --yes +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `--datasource X` | all | Limit to one datasource. | +| `--model M` | all | Limit the whole report — and `--force-clean` — to one model. | +| `--cardinality` | off | Also profile join arity (full table scans). | +| `--persist-cardinality` | off | Write the detected `cardinality` back onto each join. Implies `--cardinality`. | +| `--format` | `text` | `text` or `json`. Not combinable with `--force-clean`. | +| `--force-clean` | off | Prompt to apply each delete. Destructive. | +| `-y` / `--yes` | off | Skip the `--force-clean` prompt. | + +With a cardinality flag the output gains two labelled sections, and each join reports a `verdict` — `fills_none`, `confirms`, `refines`, `contradicts_hard`, `skipped_unsupported`, `no_evidence`, or `scan_failed`. Full table: [CLI reference](../reference/cli.md#slayer-validate-models). + ### `slayer import-dbt` Import dbt Semantic Layer definitions into SLayer. diff --git a/docs/osi/osi_import.md b/docs/osi/osi_import.md index 47c19bae..b52092c2 100644 --- a/docs/osi/osi_import.md +++ b/docs/osi/osi_import.md @@ -23,7 +23,7 @@ Spec versions `1.0`, `0.1.0`, `0.1.1`, and `0.2.0.dev0` are all accepted (they a | field `expression` (derived, e.g. `UPPER(x)`) | a derived `Column` with `sql` set | | field `dimension.is_time` | column typed temporal; sets `default_time_dimension` | | dataset `primary_key` | `Column.primary_key = true` | -| `relationships[]` (`from` → `to`) | a LEFT `ModelJoin` on the `from` model | +| `relationships[]` (`from` → `to`) | a LEFT `ModelJoin` on the `from` model, [`cardinality`](../concepts/models.md#join-cardinality) `many_to_one` (OSI relationships are direction-implied: `from` = many, `to` = one) | | `metrics[]` (raw SQL aggregation) | a `ModelMeasure` formula | | `ai_context` (instructions + synonyms) | entity `description` + `meta["osi_ai_context"]` | | `unique_keys` / `custom_extensions` | model/column `meta` | diff --git a/docs/reference/cli.md b/docs/reference/cli.md index b60eba69..6510f347 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -201,6 +201,51 @@ leave an internal out of the store entirely. An empty result prints the available schemas so a mistyped `--schema` is obvious rather than silent. + +### `slayer validate-models` + +Diff persisted models against the live database schemas (read-only) and, optionally, profile each join's arity from the data. See [Schema Drift](../concepts/schema-drift.md) and [Join cardinality](../concepts/models.md#join-cardinality). + +```bash +slayer validate-models # every datasource +slayer validate-models --datasource jaffle_shop +slayer validate-models --model orders --format json +slayer validate-models --cardinality # + profile join arity +slayer validate-models --cardinality --persist-cardinality +slayer validate-models --datasource jaffle_shop --force-clean --yes +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `--datasource X` | all | Limit to one datasource. Unknown names fail fast. | +| `--model M` | all | Limit the whole report — and `--force-clean` — to one model. Resolves across every datasource that has a model of that name. | +| `--cardinality` | off | Also profile each join's arity from the data. Full-scans both sides of every join, so it is opt-in. | +| `--persist-cardinality` | off | Write the detected `cardinality` back onto each matching join (identified by target model + key pairs). Implies `--cardinality`. | +| `--format` | `text` | `text`, or `json` for one `{"drift": [...], "cardinality": {...}}` document. Cannot be combined with `--force-clean`. | +| `--force-clean` | off | After printing the diff, prompt to apply each delete. Destructive; opt-in only. | +| `-y` / `--yes` | off | With `--force-clean`, skip the confirmation prompt. | + +Without a cardinality flag the output is the drift diff alone. With one, the report gains two labelled sections — `Schema drift` and `Join cardinality` — and any `--force-clean` apply happens *between* them, so profiling reads the repaired models. + +Exit code is `0` for any report about the data, including `contradicts_hard` — the command did its job and the answer is unwelcome. It is `1` when the command could not do its job: an unknown datasource or model, a datasource that failed validation (an unscoped run validates each one explicitly rather than silently skipping failures), a profiling failure, any `scan_failed` finding, or residual drift after `--force-clean`. Diagnostics go to stderr, so stdout stays parseable under `--format json`. + +#### Cardinality verdicts + +| Verdict | Meaning | +|---------|---------| +| `fills_none` | No cardinality was stored; the detected value fills the gap. | +| `confirms` | Detected value matches what was stored. | +| `refines` | Differs from the stored value, but the data does not disprove it — "no duplicates observed" only *suggests* uniqueness. | +| `contradicts_hard` | The data **disproves** the stored value: a side it claimed unique has duplicates. | +| `skipped_unsupported` | Not profilable — a non-`sql_table` model (sql-mode / query-backed) or an expression-valued join key. | +| `no_evidence` | Profiled fine, but one side had no non-null key rows. An empty scan says nothing about arity, so nothing is detected or written. Worth re-running once data lands — unlike `skipped_unsupported`, which never becomes profilable. | +| `scan_failed` | The scan itself raised; the message is in `note`. Contained per join, so one unreadable table costs one finding rather than the whole report — but the command still exits 1, since that join was not profiled. | + +Columns declared `unique` (or `primary_key`) that the data shows have duplicates are reported under `unique_contradictions`; detection never mutates `Column.unique`. + +Detection is a strong guess, not a guarantee: a duplicate disproves uniqueness with certainty, but its absence only suggests uniqueness. + + ### `slayer import-dbt` Import dbt Semantic Layer definitions into SLayer. diff --git a/slayer/cli.py b/slayer/cli.py index 186f1661..835c07f3 100644 --- a/slayer/cli.py +++ b/slayer/cli.py @@ -1,6 +1,7 @@ """CLI entry point for SLayer.""" import argparse +import asyncio import copy import json import os @@ -17,6 +18,7 @@ SlayerError, ) from slayer.core.models import SlayerModel +from slayer.engine.cardinality import CardinalityVerdict from slayer.engine.ingestion import ( _print_ingest_addition, _print_ingest_drift_and_errors, @@ -309,6 +311,9 @@ def main(): # NOSONAR(S3776) — linear top-level CLI command dispatch (one eli examples: slayer validate-models # check every datasource slayer validate-models --datasource my_pg + slayer validate-models --cardinality # + profile join arity (full scans) + slayer validate-models --cardinality --persist-cardinality + slayer validate-models --model orders --format json """, formatter_class=argparse.RawDescriptionHelpFormatter, ) @@ -317,6 +322,33 @@ def main(): # NOSONAR(S3776) — linear top-level CLI command dispatch (one eli default=None, help="Datasource name. If omitted, every datasource is validated.", ) + validate_parser.add_argument( + "--model", + default=None, + help="Limit the whole report (and --force-clean) to a single model.", + ) + validate_parser.add_argument( + "--cardinality", + action="store_true", + help=( + "Also profile each join's arity from the data. Full-scans both " + "sides of every join, so it is off by default." + ), + ) + validate_parser.add_argument( + "--persist-cardinality", + action="store_true", + help=( + "Write the detected cardinality back onto each matching join. " + "Implies --cardinality." + ), + ) + validate_parser.add_argument( + "--format", + default="text", + choices=["text", "json"], + help="Output format. json emits one {drift, cardinality} document.", + ) validate_parser.add_argument( "--force-clean", action="store_true", @@ -877,6 +909,11 @@ def main(): # NOSONAR(S3776) — linear top-level CLI command dispatch (one eli elif args.command == "ingest": _run_ingest(args) elif args.command == "validate-models": + if args.format == "json" and args.force_clean: + parser.error( + "--format json cannot be combined with --force-clean " + "(the apply flow is interactive and prints progress)" + ) _run_validate_models(args) elif args.command == "recommend-root-model": _run_recommend_root_model(args) @@ -1487,34 +1524,153 @@ def _format_validate_models_output(entries) -> str: return "\n".join(lines) -def _run_validate_models(args): - from slayer.engine.query_engine import SlayerQueryEngine +def _format_cardinality_report_text(report, *, indent: str = "") -> str: + if not report.findings: + return f"{indent}No joins found." + lines: list[str] = [] + for f in report.findings: + detected = f.detected.value if f.detected else "-" + stored = f.stored.value if f.stored else "-" + lines.append( + f"{indent}{f.model} -> {f.target_model}: detected={detected} " + f"stored={stored} verdict={f.verdict.value}" + ) + for c in f.unique_contradictions: + lines.append(f"{indent} ! {c}") + if f.note: + lines.append(f"{indent} ({f.note})") + return "\n".join(lines) - storage = _resolve_storage(args) + +def _indent_block(text: str, *, indent: str = " ") -> str: + return "\n".join(f"{indent}{line}" if line else line + for line in text.split("\n")) + + +def _resolve_validate_scope(args, storage) -> None: + """Fail fast on a typoed --datasource / --model. + + Either would otherwise yield an empty report, indistinguishable from + "no drift" — and exit 0. + """ + storage_path = args.storage or args.models_dir or _STORAGE_DEFAULT if args.datasource: - # Fail fast on a typoed name. Without this check, ``validate_models`` - # returns ``[]`` for an unknown datasource (no models match), which - # is indistinguishable from "no drift" and silently exits 0. - ds = run_sync(storage.get_datasource(args.datasource)) - if ds is None: - storage_path = args.storage or args.models_dir or _STORAGE_DEFAULT - print(f"Datasource '{args.datasource}' not found in {storage_path}") + if run_sync(storage.get_datasource(args.datasource)) is None: + print( + f"Datasource '{args.datasource}' not found in {storage_path}", + file=sys.stderr, + ) sys.exit(1) - engine = SlayerQueryEngine(storage=storage) + model = getattr(args, "model", None) + if not model: + return + identities = run_sync(storage._list_all_model_identities()) + # Models are keyed by (data_source, name), so an unscoped run matches the + # name in every datasource that has one. + if not any( + name == model and (not args.datasource or ds == args.datasource) + for ds, name in identities + ): + where = f"datasource '{args.datasource}'" if args.datasource else storage_path + print(f"Model '{model}' not found in {where}", file=sys.stderr) + sys.exit(1) + + +async def _validate_each_datasource(engine, ds_names: list[str]) -> list[tuple]: + """Validate every datasource concurrently, pairing each result with its name.""" + results = await asyncio.gather( + *(engine.validate_models(data_source=name) for name in ds_names), + return_exceptions=True, + ) + return list(zip(ds_names, results)) + + +def _collect_drift(args, engine, storage) -> tuple[list, list]: + """``(entries, failures)`` for the requested scope, filtered by ``--model``. + + Datasources are validated concurrently but attributed one by one: + ``validate_models(None)`` swallows a per-datasource failure, and a + validation command must not report a clean bill for a datasource it never + reached. + """ + if args.datasource: + ds_names = [args.datasource] + else: + ds_names = run_sync(storage.list_datasources()) + + entries: list = [] + failures: list[tuple[str, BaseException]] = [] + for ds_name, result in run_sync(_validate_each_datasource(engine, ds_names)): + # gather() reports a cancelled child as a CancelledError value; that is + # cancellation, not a datasource that failed validation. + if isinstance(result, asyncio.CancelledError): + raise result + if isinstance(result, BaseException): + failures.append((ds_name, result)) + else: + entries.extend(result) + return _filter_entries_by_model(entries, getattr(args, "model", None)), failures + + +def _filter_entries_by_model(entries: list, model: str | None) -> list: + if not model: + return entries + return [e for e in entries if e.model_name == model] + + +def _collect_cardinality_report(args, engine): + """Profile join arity, or ``None`` when neither cardinality flag is set.""" + persist = bool(getattr(args, "persist_cardinality", False)) + if not (persist or getattr(args, "cardinality", False)): + return None try: - entries = run_sync(engine.validate_models(data_source=args.datasource)) - except Exception as exc: # noqa: BLE001 — surface DB/auth/introspection failures cleanly - print(f"validate-models failed: {exc}") + return run_sync(engine.detect_join_cardinality( + data_source=args.datasource, + model=getattr(args, "model", None), + persist=persist, + )) + except Exception as exc: # noqa: BLE001 — surface DB/introspection failures cleanly + print(f"validate-models: cardinality profiling failed: {exc}", file=sys.stderr) sys.exit(1) - print(_format_validate_models_output(entries)) - force_clean = bool(getattr(args, "force_clean", False)) - if not force_clean: - return - if not entries: +def _exit_on_scan_failures(report) -> None: + """A contained scan failure is still work the command did not do.""" + if report is None: return + failed = [ + f for f in report.findings if f.verdict is CardinalityVerdict.SCAN_FAILED + ] + if failed: + print( + f"{len(failed)} join(s) could not be profiled; see scan_failed above.", + file=sys.stderr, + ) + sys.exit(1) + + +def _print_validate_json(*, entries, report) -> None: + import json as _json + + print(_json.dumps({ + "drift": [e.model_dump(mode="json") for e in entries], + "cardinality": report.model_dump(mode="json") if report else None, + }, indent=2)) + +def _print_drift_section(entries, *, headed: bool) -> None: + """Headers only appear when a cardinality section follows.""" + body = _format_validate_models_output(entries) + print("Schema drift\n" + _indent_block(body) if headed else body) + + +def _print_cardinality_section(report) -> None: + print("\nJoin cardinality") + print(_format_cardinality_report_text(report, indent=" ")) + + +def _apply_force_clean(args, engine, entries) -> None: + """Prompt for and apply the drift deletes; exits 1 on residual/errors.""" if not _confirm( f"\nApply {len(entries)} delete(s) to storage?", assume_yes=bool(getattr(args, "yes", False)), @@ -1528,15 +1684,61 @@ def _run_validate_models(args): print(f"Errors ({len(result.errors)}):") for err in result.errors: print(f" - {err.tool} {err.model_name}: {err.error}") - if result.residual: + # apply_drift_deletes re-validates whole datasources, so scope the residual + # the same way the report was scoped — out-of-scope drift is not ours. + residual = _filter_entries_by_model( + result.residual, getattr(args, "model", None) + ) + if residual: print("\nResidual drift after apply:") - print(_format_validate_models_output(result.residual)) + print(_format_validate_models_output(residual)) sys.exit(1) if result.errors: sys.exit(1) print("\n✓ no remaining drift") +def _run_validate_models(args): + from slayer.engine.query_engine import SlayerQueryEngine + + storage = _resolve_storage(args) + _resolve_validate_scope(args, storage) + engine = SlayerQueryEngine(storage=storage) + + entries, failures = _collect_drift(args, engine, storage) + as_json = getattr(args, "format", "text") == "json" + wants_cardinality = bool( + getattr(args, "cardinality", False) + or getattr(args, "persist_cardinality", False) + ) + + if failures: + # Report what we did reach, on stdout in the requested format, then + # fail — profiling a set we could not fully validate would mislead. + if as_json: + _print_validate_json(entries=entries, report=None) + else: + _print_drift_section(entries, headed=False) + for ds_name, exc in failures: + print(f"Datasource '{ds_name}' failed validation: {exc}", file=sys.stderr) + sys.exit(1) + + if as_json: + report = _collect_cardinality_report(args, engine) + _print_validate_json(entries=entries, report=report) + _exit_on_scan_failures(report) + return + + _print_drift_section(entries, headed=wants_cardinality) + # Clean first: profiling a model we are about to repair is wasted work. + if bool(getattr(args, "force_clean", False)) and entries: + _apply_force_clean(args, engine, entries) + report = _collect_cardinality_report(args, engine) + if report is not None: + _print_cardinality_section(report) + _exit_on_scan_failures(report) + + def _run_recommend_root_model(args): import json as _json diff --git a/slayer/core/enums.py b/slayer/core/enums.py index af1ca54e..afbe85e5 100644 --- a/slayer/core/enums.py +++ b/slayer/core/enums.py @@ -157,6 +157,31 @@ class JoinType(StrEnum): INNER = "inner" +class JoinCardinality(StrEnum): + """Arity of a join, read source->target ("many orders -> one customer"). + + Descriptive metadata, orthogonal to ``JoinType`` — it does not change the + emitted join. + """ + ONE_TO_ONE = "one_to_one" + ONE_TO_MANY = "one_to_many" + MANY_TO_ONE = "many_to_one" + MANY_TO_MANY = "many_to_many" + + +def invert_cardinality( + cardinality: "JoinCardinality | None", +) -> "JoinCardinality | None": + """Cardinality of the reverse edge; one_to_one / many_to_many / None are + self-inverse. + """ + if cardinality is JoinCardinality.MANY_TO_ONE: + return JoinCardinality.ONE_TO_MANY + if cardinality is JoinCardinality.ONE_TO_MANY: + return JoinCardinality.MANY_TO_ONE + return cardinality + + # The kind of database object a ``sql_table``-mode model points at. A plain # ``Literal`` (not ``StrEnum``) so the values persist as bare strings in # YAML/SQLite without an enum-serialisation round-trip. diff --git a/slayer/core/models.py b/slayer/core/models.py index 2563e692..87dbf11a 100644 --- a/slayer/core/models.py +++ b/slayer/core/models.py @@ -11,6 +11,7 @@ from slayer.core.enums import ( BUILTIN_AGGREGATIONS, DataType, + JoinCardinality, JoinType, ObjectKind, _coerce_legacy_datatype, @@ -200,6 +201,7 @@ class Column(BaseModel): ), ) primary_key: bool = False + unique: bool = False # single-column uniqueness (non-PK); primary_key implies it description: str | None = None label: str | None = None hidden: bool = False @@ -454,6 +456,8 @@ class ModelJoin(BaseModel): target_model: str # Name of the joined model join_pairs: list[list[str]] = Field(...) # [["source_dim", "target_dim"], ...] join_type: JoinType = JoinType.LEFT # LEFT (default) or INNER + # Join arity, read source->target; None = undetermined. + cardinality: JoinCardinality | None = None # DEV-1643: optional human/agent metadata (e.g. carrying OSI relationship # ai_context on import). Purely additive/optional — old data omits them and # validates unchanged, so no SlayerModel schema-version bump is needed. diff --git a/slayer/dbt/converter.py b/slayer/dbt/converter.py index ca3ad7ae..f6a57ca1 100644 --- a/slayer/dbt/converter.py +++ b/slayer/dbt/converter.py @@ -22,7 +22,7 @@ import sqlalchemy as sa -from slayer.core.enums import DataType, JoinType +from slayer.core.enums import DataType, JoinType, invert_cardinality from slayer.core.format import NumberFormat, NumberFormatType from slayer.core.formula import parse_formula from slayer.core.models import Column, ModelJoin, ModelMeasure, SlayerModel @@ -218,6 +218,7 @@ def _mirror_inner_joins(self) -> None: target_model=model.name, join_pairs=reverse_pairs, join_type=JoinType.INNER, + cardinality=invert_cardinality(join.cardinality), )) def _prune_dangling_measures(self) -> None: diff --git a/slayer/dbt/entities.py b/slayer/dbt/entities.py index e794c355..9fd1c374 100644 --- a/slayer/dbt/entities.py +++ b/slayer/dbt/entities.py @@ -7,7 +7,7 @@ import logging -from slayer.core.enums import JoinType +from slayer.core.enums import JoinCardinality, JoinType from slayer.core.models import ModelJoin from slayer.dbt.models import DbtSemanticModel @@ -110,6 +110,8 @@ def resolve_joins_for_model(self, model: DbtSemanticModel) -> list[ModelJoin]: target_model=target_model_name, join_pairs=[[foreign_expr, primary_expr]], join_type=JoinType.INNER, + # foreign -> primary: many source rows, one target row. + cardinality=JoinCardinality.MANY_TO_ONE, )) # Peer joins: models sharing the same primary/unique entity are joinable @@ -130,6 +132,8 @@ def resolve_joins_for_model(self, model: DbtSemanticModel) -> list[ModelJoin]: target_model=peer_model_name, join_pairs=[[local_expr, peer_expr]], join_type=JoinType.INNER, + # Both sides are a primary/unique entity: one-to-one. + cardinality=JoinCardinality.ONE_TO_ONE, )) return joins diff --git a/slayer/engine/cardinality.py b/slayer/engine/cardinality.py new file mode 100644 index 00000000..f39a2814 --- /dev/null +++ b/slayer/engine/cardinality.py @@ -0,0 +1,139 @@ +"""Join-cardinality inference helpers and the detection report. + +Uniqueness is asymmetric evidence: a scan disproves it with one duplicate, but +can never prove it. +""" + +from __future__ import annotations + +from pydantic import BaseModel, Field + +from slayer.core.enums import JoinCardinality, StrEnum + + +# --------------------------------------------------------------------------- +# Pure inference +# --------------------------------------------------------------------------- + + +def is_key_set_unique( + *, key_columns: list[str], unique_key_sets: list[list[str]] +) -> bool: + """Is the ``key_columns`` tuple unique given the known unique key-sets? + + Unique iff some key-set is a non-empty SUBSET: unique ``(a)`` makes + ``(a, b)`` unique, but unique ``(a, b)`` says nothing about ``(a)``. + """ + key_set = set(key_columns) + for uks in unique_key_sets: + if uks and set(uks) <= key_set: + return True + return False + + +def declares_solo_unique(*, columns, column) -> bool: + """Does ``column`` ALONE carry a declared uniqueness among ``columns``? + + ``primary_key`` is stamped on every member of a composite PK, so it + implies solo uniqueness only when the column IS the whole primary key. + """ + if column.unique: + return True + return column.primary_key and sum(1 for c in columns if c.primary_key) == 1 + + +def classify_cardinality( + *, source_unique: bool, target_unique: bool +) -> JoinCardinality: + """Total classification from the two sides' uniqueness (profiling path).""" + if source_unique and target_unique: + return JoinCardinality.ONE_TO_ONE + if target_unique: + return JoinCardinality.MANY_TO_ONE + if source_unique: + return JoinCardinality.ONE_TO_MANY + return JoinCardinality.MANY_TO_MANY + + +def infer_structural_cardinality( + *, source_unique: bool, target_verified_unique: bool +) -> JoinCardinality | None: + """Ingest-time guess. ``None`` unless the target is *verified* unique.""" + if not target_verified_unique: + return None + return classify_cardinality(source_unique=source_unique, target_unique=True) + + +def _claims_source_unique(c: JoinCardinality) -> bool: + return c in (JoinCardinality.ONE_TO_ONE, JoinCardinality.ONE_TO_MANY) + + +def _claims_target_unique(c: JoinCardinality) -> bool: + return c in (JoinCardinality.ONE_TO_ONE, JoinCardinality.MANY_TO_ONE) + + +# --------------------------------------------------------------------------- +# Detection report (Pydantic, list-of-named-entries — no dict fields) +# --------------------------------------------------------------------------- + + +class CardinalityVerdict(StrEnum): + CONFIRMS = "confirms" + REFINES = "refines" + FILLS_NONE = "fills_none" + CONTRADICTS_HARD = "contradicts_hard" + SKIPPED_UNSUPPORTED = "skipped_unsupported" + #: Profiled fine but the key population was empty — an empty scan is no + #: evidence of arity. Unlike SKIPPED_UNSUPPORTED, worth re-running later. + NO_EVIDENCE = "no_evidence" + #: The scan itself failed; the message is in ``note``. Contained per join + #: so one unreadable table cannot abort the whole report. + SCAN_FAILED = "scan_failed" + + +class SideStats(BaseModel): + row_count: int # non-null key rows + distinct_count: int + observed_unique: bool + + +class JoinCardinalityFinding(BaseModel): + data_source: str + model: str + target_model: str + join_pairs: list[list[str]] + stored: JoinCardinality | None = None + detected: JoinCardinality | None = None + source_side: SideStats | None = None + target_side: SideStats | None = None + verdict: CardinalityVerdict + unique_contradictions: list[str] = Field(default_factory=list) + note: str | None = None + + +class JoinCardinalityReport(BaseModel): + findings: list[JoinCardinalityFinding] = Field(default_factory=list) + + +def compute_verdict( + *, + stored: JoinCardinality | None, + detected: JoinCardinality, + source_observed_unique: bool, + target_observed_unique: bool, +) -> CardinalityVerdict: + """Classify a detected value against the stored one. + + ``CONTRADICTS_HARD`` only when the data disproves a uniqueness the stored + value asserted; every other mismatch is a soft ``REFINES``. + """ + if stored is None: + return CardinalityVerdict.FILLS_NONE + if detected == stored: + return CardinalityVerdict.CONFIRMS + hard = (_claims_source_unique(stored) and not source_observed_unique) or ( + _claims_target_unique(stored) and not target_observed_unique + ) + return ( + CardinalityVerdict.CONTRADICTS_HARD if hard else CardinalityVerdict.REFINES + ) diff --git a/slayer/engine/ingestion.py b/slayer/engine/ingestion.py index b50623a6..4e3948cd 100644 --- a/slayer/engine/ingestion.py +++ b/slayer/engine/ingestion.py @@ -27,6 +27,10 @@ SlayerModel, sanitize_model_name, ) +from slayer.engine.cardinality import ( + infer_structural_cardinality, + is_key_set_unique, +) from slayer.engine.internal_tables import internal_table_rule from slayer.engine.introspect_utils import ( # noqa: F401 (re-exported for back-compat) _FLOAT_LIKE_INFO_SCHEMA_TYPES, @@ -306,6 +310,25 @@ class RollupGraphError(Exception): # --------------------------------------------------------------------------- +def _is_cross_schema_fk( + fk: dict, schema: str | None, default_schema: str | None = None, +) -> bool: + """Does this FK point at a table outside the schema being ingested? + + Models are keyed by bare table name, so a cross-schema FK has no model to + bind to and would otherwise bind to a same-named local table. + """ + referred_schema = fk.get("referred_schema") + if referred_schema is None: # same-schema FK; always None on SQLite + return False + # Ingesting the default schema passes schema=None, so fall back to it. + effective_schema = schema if schema is not None else default_schema + # Unknown ingested schema: skip rather than risk binding to the wrong table. + if effective_schema is None: + return True + return referred_schema != effective_schema + + def _get_fk_relationships( inspector: sa.engine.Inspector, table_name: str, @@ -330,6 +353,12 @@ def _get_fk_relationships( referred_table = fk["referred_table"] if referred_table not in table_set or referred_table == table_name: continue + if _is_cross_schema_fk( + fk=fk, + schema=schema, + default_schema=getattr(inspector, "default_schema_name", None), + ): + continue constrained = fk["constrained_columns"] referred = fk["referred_columns"] for src_col, tgt_col in zip(constrained, referred): @@ -403,39 +432,256 @@ def _compute_transitive_closure(graph: dict[str, set[str]], source: str) -> set[ # --------------------------------------------------------------------------- +def _get_fk_constraint_groups( + inspector: sa.engine.Inspector, + table_name: str, + schema: str | None, + table_set: set[str], +) -> list[tuple[str, list[tuple[str, str]]]]: + """``[(referred_table, [(src_col, tgt_col), ...]), ...]``, one entry per FK + constraint — so a composite FK stays one grouped join. + """ + fks = inspector.get_foreign_keys(table_name, schema=schema) + result: list[tuple[str, list[tuple[str, str]]]] = [] + for fk in fks: + referred_table = fk["referred_table"] + if referred_table not in table_set or referred_table == table_name: + continue + if _is_cross_schema_fk( + fk=fk, + schema=schema, + default_schema=getattr(inspector, "default_schema_name", None), + ): + continue + pairs = list(zip(fk["constrained_columns"], fk["referred_columns"])) + if pairs: + result.append((referred_table, pairs)) + return result + + +def _safe_introspect(fn) -> list: + """Run a best-effort introspection call, yielding ``[]`` on failure. + + Constraint/index reflection is unsupported or partial on several backends, + so a raising call degrades to "no uniqueness evidence" rather than aborting. + """ + try: + return list(fn()) + except Exception as exc: # noqa: BLE001 — degrade to "no evidence" + logger.debug( + "Constraint/index reflection unavailable (%s); " + "treating as no uniqueness evidence.", exc, + ) + return [] + + +def _pk_key_sets( + inspector: sa.engine.Inspector, + table_name: str, + schema: str | None, + sa_engine: sa.Engine | None, +) -> list[list[str]]: + """The table's primary key as a single key-set (or none).""" + try: + if sa_engine is not None: + pk = _safe_get_pk_constraint( + inspector=inspector, + sa_engine=sa_engine, + table_name=table_name, + schema=schema, + ) + else: + # Only the bare-inspector path needs normalizing; + # _safe_get_pk_constraint already guarantees a mapping. + pk = inspector.get_pk_constraint(table_name=table_name, schema=schema) + if not isinstance(pk, dict): + return [] + except Exception: + return [] + cols = pk.get("constrained_columns") + return [list(cols)] if cols else [] + + +def _unique_constraint_key_sets( + inspector: sa.engine.Inspector, table_name: str, schema: str | None, +) -> list[list[str]]: + """Key-sets from declared UNIQUE constraints.""" + out: list[list[str]] = [] + for uc in _safe_introspect( + lambda: inspector.get_unique_constraints(table_name, schema=schema) + ): + cols = uc.get("column_names") or [] + if cols and all(cols): + out.append(list(cols)) + return out + + +def _is_partial_index(idx: dict) -> bool: + """Does this index carry a filter predicate (a PARTIAL index)? + + A partial unique index constrains only the rows matching its predicate, so + it is no evidence of whole-table uniqueness. + """ + opts = idx.get("dialect_options") or {} + for key, value in opts.items(): + # SQLAlchemy names the predicate per dialect: postgresql_where, ... + if not key.endswith("_where") or value is None: + continue + if isinstance(value, str): + if value.strip(): + return True + continue + # Never bool() a non-string: ColumnElement.__bool__ raises, and this + # runs outside _safe_introspect. Presence alone means "predicate". + return True + return False + + +def _unique_index_key_sets( + inspector: sa.engine.Inspector, table_name: str, schema: str | None, +) -> list[list[str]]: + """Key-sets from unique indexes that constrain the WHOLE table.""" + out: list[list[str]] = [] + for idx in _safe_introspect( + lambda: inspector.get_indexes(table_name, schema=schema) + ): + if not idx.get("unique") or _is_partial_index(idx): + continue + cols = idx.get("column_names") or [] + # Expression members reflect as None, so any falsy member rejects the + # whole set — else unique(email, lower(name)) claims email alone. + if cols and all(cols): + out.append(list(cols)) + return out + + +def _get_unique_key_sets( + inspector: sa.engine.Inspector, + table_name: str, + schema: str | None, + sa_engine: sa.Engine | None = None, +) -> list[list[str]]: + """All PK + UNIQUE key-sets for a table.""" + return ( + _pk_key_sets( + inspector=inspector, table_name=table_name, + schema=schema, sa_engine=sa_engine, + ) + + _unique_constraint_key_sets( + inspector=inspector, table_name=table_name, schema=schema, + ) + + _unique_index_key_sets( + inspector=inspector, table_name=table_name, schema=schema, + ) + ) + + +def _solo_unique_columns_for_table( + *, + inspector: sa.engine.Inspector, + sa_engine: sa.Engine, + table_name: str, + schema: str | None, +) -> set[str]: + """``_get_single_column_unique_names`` with the table's PK resolved for it.""" + pk = _safe_get_pk_constraint( + inspector=inspector, sa_engine=sa_engine, + table_name=table_name, schema=schema, + ) + return _get_single_column_unique_names( + inspector=inspector, table_name=table_name, schema=schema, + pk_cols=set(pk.get("constrained_columns", [])), + ) + + +def _get_single_column_unique_names( + inspector: sa.engine.Inspector, + table_name: str, + schema: str | None, + *, + pk_cols: set[str], +) -> set[str]: + """Names of columns that ALONE form a UNIQUE constraint / unique index. + + PK columns are excluded (``primary_key`` is the canonical marker), and so + are composite key-sets — unique ``(a, b)`` says nothing about ``a`` alone. + """ + key_sets = ( + _unique_constraint_key_sets( + inspector=inspector, table_name=table_name, schema=schema, + ) + + _unique_index_key_sets( + inspector=inspector, table_name=table_name, schema=schema, + ) + ) + names = {ks[0] for ks in key_sets if len(ks) == 1} + return names - set(pk_cols) + + def _generate_joins( inspector: sa.engine.Inspector, source_table: str, referenced_tables: set[str], schema: str | None, table_set: set[str], + sa_engine: sa.Engine | None = None, + model_name_by_table: dict[str, str] | None = None, ) -> list[ModelJoin]: - """Generate direct ModelJoin objects from the source table's own FK relationships. + """Direct ModelJoins from the source table's own FKs (multi-hop is resolved + at query time). A composite FK becomes one grouped join, and cardinality is + inferred from key constraints alone. - Only emits joins for FKs defined on ``source_table`` itself — multi-hop - reachability (e.g. orders → customers → regions) is resolved at query time - by walking the join graph through each intermediate model. + ``model_name_by_table`` maps live object names to the model names they were + ingested under; a join names the MODEL, and a target with no model is + dropped rather than left dangling. """ - fk_rels = _get_fk_relationships( + groups = _get_fk_constraint_groups( inspector=inspector, table_name=source_table, schema=schema, table_set=table_set, ) + source_uniques = _get_unique_key_sets( + inspector=inspector, table_name=source_table, + schema=schema, sa_engine=sa_engine, + ) joins = [] - seen_signatures: set[tuple[str, str, str]] = set() - for src_col, ref_table, tgt_col in fk_rels: + seen_signatures: set[tuple] = set() + for ref_table, pairs in groups: if ref_table not in referenced_tables: continue - signature = (ref_table, src_col, tgt_col) + # Model names strip `__`, so the live name is not always the model name. + target_name = ( + ref_table if model_name_by_table is None + else model_name_by_table.get(ref_table) + ) + if target_name is None: + continue # target collided on sanitization and was never ingested + signature = (ref_table, tuple(pairs)) if signature in seen_signatures: continue seen_signatures.add(signature) + + source_cols = [s for s, _ in pairs] + target_cols = [t for _, t in pairs] + target_uniques = _get_unique_key_sets( + inspector=inspector, table_name=ref_table, + schema=schema, sa_engine=sa_engine, + ) + cardinality = infer_structural_cardinality( + source_unique=is_key_set_unique( + key_columns=source_cols, unique_key_sets=source_uniques + ), + target_verified_unique=is_key_set_unique( + key_columns=target_cols, unique_key_sets=target_uniques + ), + ) joins.append( ModelJoin( - target_model=ref_table, - join_pairs=[[src_col, tgt_col]], + target_model=target_name, + join_pairs=[[s, t] for s, t in pairs], + cardinality=cardinality, ) ) @@ -490,15 +736,15 @@ def _safe_get_pk_constraint( ) -> dict: """Get PK constraint, falling back to INFORMATION_SCHEMA on failure. - SQLite has no information_schema views; its stock inspector reads - PRAGMA table_info() and is authoritative — empty constrained_columns - on SQLite means the table genuinely has no primary key. + ALWAYS returns a mapping — the one place normalizing an inspector that may + hand back ``None``. SQLite's PRAGMA reflection is authoritative. """ if sa_engine.dialect.name == "sqlite": try: - return inspector.get_pk_constraint(table_name, schema=schema) + result = inspector.get_pk_constraint(table_name=table_name, schema=schema) except Exception: return {"constrained_columns": []} + return result if isinstance(result, dict) else {"constrained_columns": []} try: result = inspector.get_pk_constraint(table_name, schema=schema) if result.get("constrained_columns"): @@ -518,6 +764,7 @@ def _introspect_query_columns_via_inspector( referenced_tables: set[str], fk_columns_by_table: dict[str, set[str]], joins: list[ModelJoin] | None = None, + live_name_by_model: dict[str, str] | None = None, ) -> list[tuple]: """Introspect columns from a rollup query or plain table. @@ -555,14 +802,21 @@ def _introspect_query_columns_via_inspector( # Build list of (ref_table, dotted_path) from joins — supports diamond joins # where the same table appears via multiple paths table_path_pairs: list[tuple] = [] - if joins: + # `is not None`, not truthiness: an EMPTY join list means every candidate + # join was dropped, which is not the same as "joins were never generated". + if joins is not None: + lookup = live_name_by_model or {} for mj in joins: if mj.join_pairs and "." in mj.join_pairs[0][0]: prefix = mj.join_pairs[0][0].split(".")[0] path = f"{prefix}.{mj.target_model}" else: path = mj.target_model - table_path_pairs.append((mj.target_model, path)) + # The path alias is the MODEL name; introspection needs the live + # object name, and sanitization can make the two differ. + table_path_pairs.append( + (lookup.get(mj.target_model, mj.target_model), path) + ) else: # Fallback: one entry per referenced table for ref_table in referenced_tables: @@ -606,6 +860,7 @@ def _columns_to_model( data_source: str, sql_table: str | None = None, joins: list[ModelJoin] | None = None, + unique_columns: set[str] | None = None, source_kind: ObjectKind | None = None, hidden: bool = False, meta: dict[str, Any] | None = None, @@ -619,6 +874,7 @@ def _columns_to_model( carried through verbatim (set only for opaque ``UNKNOWN`` columns). """ cols: list[Column] = [] + unique_set = unique_columns or set() _INT_FORMAT = NumberFormat(type=NumberFormatType.INTEGER) _FLOAT_FORMAT = NumberFormat(type=NumberFormatType.FLOAT) @@ -647,6 +903,7 @@ def _columns_to_model( type=data_type, db_type=db_type, primary_key=is_pk, + unique=(col_name in unique_set), format=fmt, ) ) @@ -774,11 +1031,16 @@ def introspect_table_to_model( sql_table=sql_table, columns=columns, ) + unique_columns = _solo_unique_columns_for_table( + inspector=inspector, sa_engine=sa_engine, + table_name=table_name, schema=schema, + ) return _columns_to_model( name=model_name or table_name, columns=columns, data_source=data_source, sql_table=sql_table, + unique_columns=unique_columns, source_kind=source_kind, ) @@ -974,6 +1236,8 @@ def _build_one_model( has_cycles: bool, fk_columns_by_table: dict[str, set[str]], table_set: set[str], + model_name_by_table: dict[str, str] | None = None, + live_name_by_model: dict[str, str] | None = None, internal_tool: str | None = None, ) -> SlayerModel: """Introspect one live object into a model. Raises on failure; the caller @@ -997,6 +1261,8 @@ def _build_one_model( referenced_tables=referenced, schema=schema, table_set=table_set, + sa_engine=sa_engine, + model_name_by_table=model_name_by_table, ) columns = _introspect_query_columns_via_inspector( @@ -1008,6 +1274,7 @@ def _build_one_model( referenced_tables=referenced, fk_columns_by_table=fk_columns_by_table, joins=model_joins, + live_name_by_model=live_name_by_model, ) columns = _sqlite_probe_integer_columns( sa_engine=sa_engine, @@ -1021,6 +1288,10 @@ def _build_one_model( data_source=data_source, sql_table=sql_table, joins=model_joins, + unique_columns=_solo_unique_columns_for_table( + inspector=inspector, sa_engine=sa_engine, + table_name=obj.name, schema=schema, + ), source_kind=obj.kind, hidden=internal_tool is not None, meta=meta, @@ -1099,6 +1370,7 @@ def ingest_datasource_report( table_set = set(table_names) name_by_object, skipped = _assign_model_names(objects) + live_by_model = {model: live for live, model in name_by_object.items()} # Build FK graph, check for cycles fk_graph = _build_fk_graph( @@ -1137,6 +1409,8 @@ def ingest_datasource_report( has_cycles=has_cycles, fk_columns_by_table=fk_columns_by_table, table_set=table_set, + model_name_by_table=name_by_object, + live_name_by_model=live_by_model, internal_tool=None if surface_internals else tool, ) ) @@ -1266,15 +1540,78 @@ def _merge_persisted_column_with_probe( return persisted_col.model_copy(update=updates), True +def _join_sig(j: ModelJoin) -> tuple: + return (j.target_model, tuple(sorted((p[0], p[1]) for p in j.join_pairs))) + + +def _repair_legacy_join_targets( + persisted: SlayerModel, fresh: SlayerModel, +) -> tuple[SlayerModel, bool]: + """Rewrite persisted join targets that name the live object, not the model. + + Ingests before the sanitizing fix wrote the raw table name, and a model + name can never contain ``__`` — so such a target resolves to nothing. + + The repair demands a full signature match (sanitized target AND identical + ``join_pairs``) against a fresh join, so it can only ever rename the join + the bug produced. Matching on the target alone would repoint a + hand-authored join with different pairs, and could collapse two legacy + targets (``a__b`` and ``a___b`` both sanitize to ``a_b``) onto one name — + tripping the duplicate-target guard below and turning a tolerated store + into a failed re-ingest. + """ + fresh_sigs = {_join_sig(j) for j in fresh.joins} + claimed = { + j.target_model for j in persisted.joins + if sanitize_model_name(j.target_model) == j.target_model + } + repaired = False + joins: list[ModelJoin] = [] + for j in persisted.joins: + candidate = sanitize_model_name(j.target_model) + renamed = j.model_copy(update={"target_model": candidate}) + if ( + candidate != j.target_model + and candidate not in claimed + and _join_sig(renamed) in fresh_sigs + ): + joins.append(renamed) + claimed.add(candidate) + repaired = True + else: + joins.append(j) + if not repaired: + return persisted, False + return persisted.model_copy(update={"joins": joins}), True + + def _merge_joins_strict( persisted: SlayerModel, fresh: SlayerModel, -) -> tuple[list[ModelJoin], list[str]]: +) -> tuple[list[ModelJoin], list[str], bool]: """Append joins whose signature isn't already present. Raises on the duplicate-target / different-pairs conflict so callers don't end up - with two joins pointing at the same target_model.""" + with two joins pointing at the same target_model. + + An unset ``cardinality`` is filled from the matching fresh join; a + user-set one is never overwritten. The third return value flags a + metadata-only fill, which still has to trigger a save. + """ + persisted, target_repaired = _repair_legacy_join_targets(persisted, fresh) + existing_join_sigs = _existing_join_signatures(persisted) existing_join_targets = {j.target_model for j in persisted.joins} - new_joins: list[ModelJoin] = list(persisted.joins) + fresh_by_sig = {_join_sig(j): j for j in fresh.joins} + + metadata_changed = target_repaired + new_joins: list[ModelJoin] = [] + for pj in persisted.joins: + fj = fresh_by_sig.get(_join_sig(pj)) + if pj.cardinality is None and fj is not None and fj.cardinality is not None: + new_joins.append(pj.model_copy(update={"cardinality": fj.cardinality})) + metadata_changed = True + else: + new_joins.append(pj) + new_join_targets: list[str] = [] for j in fresh.joins: sig = (j.target_model, tuple(sorted((p[0], p[1]) for p in j.join_pairs))) @@ -1291,7 +1628,7 @@ def _merge_joins_strict( ) new_joins.append(j) new_join_targets.append(j.target_model) - return new_joins, new_join_targets + return new_joins, new_join_targets, metadata_changed class AdditiveMergeResult(BaseModel): @@ -1302,6 +1639,9 @@ class AdditiveMergeResult(BaseModel): new_joins: list[str] = Field(default_factory=list) widened_columns: list[str] = Field(default_factory=list) kind_changed: bool = False + #: A metadata-only fill (join cardinality / column unique) that still + #: has to be saved even when no column or join was added. + metadata_changed: bool = False def _additive_merge_existing( @@ -1335,6 +1675,7 @@ def _additive_merge_existing( widened_column_names: list[str] = [] merged_columns: list[Column] = [] + metadata_changed = False for persisted_col in persisted.columns: merged_col, did_widen = _merge_persisted_column_with_probe( persisted_col=persisted_col, @@ -1342,6 +1683,11 @@ def _additive_merge_existing( model_name=persisted.name, sqlite_widen_enabled=sqlite_widen_enabled, ) + # Set `unique` additively — never downgrade a user-set flag. + fresh_col = fresh_by_name.get(persisted_col.name) + if fresh_col is not None and fresh_col.unique and not merged_col.unique: + merged_col = merged_col.model_copy(update={"unique": True}) + metadata_changed = True merged_columns.append(merged_col) if did_widen: widened_column_names.append(persisted_col.name) @@ -1353,7 +1699,10 @@ def _additive_merge_existing( merged_columns.append(fresh_col) new_column_names.append(fresh_col.name) - new_joins, new_join_targets = _merge_joins_strict(persisted, fresh) + new_joins, new_join_targets, joins_metadata_changed = _merge_joins_strict( + persisted, fresh + ) + metadata_changed = metadata_changed or joins_metadata_changed # In the short-circuit below (not just the update dict), else a view→table # flip that changes nothing else would never reach the refresh. @@ -1367,6 +1716,7 @@ def _additive_merge_existing( or new_join_targets or widened_column_names or kind_changed + or metadata_changed ): return AdditiveMergeResult(merged=persisted) @@ -1380,6 +1730,7 @@ def _additive_merge_existing( new_joins=new_join_targets, widened_columns=widened_column_names, kind_changed=kind_changed, + metadata_changed=metadata_changed, ) @@ -1416,13 +1767,15 @@ async def _process_one_table( fresh=fresh, sqlite_widen_enabled=(datasource.type or "").lower() == "sqlite", ) - # ``kind_changed`` gates the save too — a view→table flip usually adds no - # columns or joins, so otherwise the refreshed model would be discarded. + # ``kind_changed`` and ``metadata_changed`` gate the save too: a view→table + # flip, or a cardinality / unique fill, usually adds no columns or joins, so + # otherwise the refreshed model would be discarded. if ( outcome.new_columns or outcome.new_joins or outcome.widened_columns or outcome.kind_changed + or outcome.metadata_changed ): await storage.save_model(outcome.merged) kind_change = None diff --git a/slayer/engine/query_engine.py b/slayer/engine/query_engine.py index 42828fc3..34d3be69 100644 --- a/slayer/engine/query_engine.py +++ b/slayer/engine/query_engine.py @@ -19,13 +19,22 @@ import sqlalchemy as sa from sqlglot import exp -from slayer.core.enums import DEFAULT_AGGREGATIONS_BY_TYPE, DataType +from slayer.core.enums import DEFAULT_AGGREGATIONS_BY_TYPE, DataType, JoinCardinality from slayer.core.errors import ( AmbiguousModelError, ForcedFilterError, IdentifierCollisionError, UnresolvableDimensionJoinError, ) +from slayer.engine.cardinality import ( + CardinalityVerdict, + JoinCardinalityFinding, + JoinCardinalityReport, + SideStats, + classify_cardinality, + compute_verdict, + declares_solo_unique, +) from slayer.core.policy import JoinFilterRuleset, SessionPolicy from slayer.core.format import NumberFormat, NumberFormatType, format_number from slayer.core.models import ( @@ -2522,6 +2531,254 @@ async def apply_drift_deletes( residual=list(residual), ) + async def detect_join_cardinality( + self, + *, + data_source: str | None = None, + model: str | None = None, + persist: bool = False, + ) -> JoinCardinalityReport: + """Profile each join's two sides and classify its cardinality. + + Full-scans non-null key rows vs distinct key-tuples, report-only unless + ``persist``. Only ``sql_table`` models with bare-column join keys are + profiled; the rest report ``SKIPPED_UNSUPPORTED``. + """ + ds_names = ( + [data_source] if data_source + else await self.storage.list_datasources() + ) + findings: list[JoinCardinalityFinding] = [] + persist_map: dict[tuple[str, str], list[tuple]] = {} + + for ds_name in ds_names: + ds_findings, ds_persist = await self._detect_datasource_joins( + ds_name=ds_name, model=model, + ) + findings.extend(ds_findings) + for model_name, signature, detected in ds_persist: + persist_map.setdefault((ds_name, model_name), []).append( + (signature, detected) + ) + + if persist: + for (ds_name, model_name), items in persist_map.items(): + await self._persist_join_cardinality( + data_source=ds_name, model_name=model_name, items=items, + ) + return JoinCardinalityReport(findings=findings) + + async def _resolve_detection_scope(self, *, ds_name, model): + """In-scope models for one datasource, plus the name lookup for joins. + + The lookup spans the whole datasource even when ``model`` narrows the + scope — join targets must still resolve. + """ + all_models = await _all_models_in_datasource(self.storage, ds_name) + by_name = {m.name: m for m in all_models} + if model is None: + return all_models, by_name + return ([by_name[model]] if model in by_name else []), by_name + + async def _detect_datasource_joins( + self, *, ds_name, model, + ) -> "tuple[list[JoinCardinalityFinding], list[tuple]]": + """Profile every join of every in-scope model in ONE datasource. + + Returns ``(findings, persist_entries)``, each persist entry being + ``(model_name, join_signature, detected)``. + """ + scope, by_name = await self._resolve_detection_scope( + ds_name=ds_name, model=model, + ) + if not scope: + return [], [] + ds_cfg = await self.storage.get_datasource(ds_name) + if ds_cfg is None: + return [], [] + + sqlglot_name = dialect_for_ds_type(ds_cfg.type).sqlglot_name + findings: list[JoinCardinalityFinding] = [] + persist_entries: list[tuple] = [] + client = SlayerSQLClient(datasource=ds_cfg) + try: + for m in scope: + for join in m.joins: + try: + finding, detected = await self._detect_one_join( + model=m, join=join, by_name=by_name, + client=client, sqlglot_name=sqlglot_name, + data_source=ds_name, datasource_cfg=ds_cfg, + ) + except ForcedFilterError: + # The session policy is fail-closed: downgrading it to + # a report line would hand back unscoped statistics. + raise + except Exception as exc: # noqa: BLE001 + # Contain per join: one unreadable table must not cost + # the whole report. + findings.append( + _scan_failed_finding( + data_source=ds_name, model=m, join=join, exc=exc, + ) + ) + continue + findings.append(finding) + if detected is not None: + persist_entries.append( + (m.name, _join_signature(join), detected) + ) + finally: + await client.aclose() + return findings, persist_entries + + async def _detect_one_join( + self, *, model, join, by_name, client, sqlglot_name, data_source, + datasource_cfg, + ) -> "tuple[JoinCardinalityFinding, JoinCardinality | None]": + pairs = [[p[0], p[1]] for p in join.join_pairs] + src_cols = [p[0] for p in join.join_pairs] + tgt_cols = [p[1] for p in join.join_pairs] + target = by_name.get(join.target_model) + + note = _detection_skip_reason( + model=model, target=target, src_cols=src_cols, tgt_cols=tgt_cols, + ) + if note is not None: + return JoinCardinalityFinding( + data_source=data_source, model=model.name, + target_model=join.target_model, join_pairs=pairs, + stored=join.cardinality, detected=None, + verdict=CardinalityVerdict.SKIPPED_UNSUPPORTED, note=note, + ), None + + src_side = await self._side_stats( + client=client, table=model.sql_table, + key_cols=src_cols, sqlglot_name=sqlglot_name, + datasource=datasource_cfg, + ) + tgt_side = await self._side_stats( + client=client, table=target.sql_table, + key_cols=tgt_cols, sqlglot_name=sqlglot_name, + datasource=datasource_cfg, + ) + # 0 == 0 would read as observed_unique, so an empty side would + # "detect" one_to_one and persist it. No rows is no evidence. + empty_sides = [ + name + for name, side in (("source", src_side), ("target", tgt_side)) + if side.row_count == 0 + ] + if empty_sides: + return JoinCardinalityFinding( + data_source=data_source, model=model.name, + target_model=join.target_model, join_pairs=pairs, + stored=join.cardinality, detected=None, + source_side=src_side, target_side=tgt_side, + verdict=CardinalityVerdict.NO_EVIDENCE, + note=( + f"no non-null key rows on the {' and '.join(empty_sides)} " + f"side; an empty scan is no evidence of arity" + ), + ), None + + detected = classify_cardinality( + source_unique=src_side.observed_unique, + target_unique=tgt_side.observed_unique, + ) + verdict = compute_verdict( + stored=join.cardinality, detected=detected, + source_observed_unique=src_side.observed_unique, + target_observed_unique=tgt_side.observed_unique, + ) + contradictions = _unique_contradictions( + model=model, target=target, src_cols=src_cols, tgt_cols=tgt_cols, + src_side=src_side, tgt_side=tgt_side, + ) + return JoinCardinalityFinding( + data_source=data_source, model=model.name, + target_model=join.target_model, join_pairs=pairs, + stored=join.cardinality, detected=detected, + source_side=src_side, target_side=tgt_side, + verdict=verdict, unique_contradictions=contradictions, + ), detected + + @staticmethod + def _side_stats_sql(*, table, key_cols, sqlglot_name) -> tuple[str, str]: + """Build the (row-count, distinct-count) profiling SQL via sqlglot. + + NULL key rows are excluded from BOTH counts, so the two are computed + over the same population. + """ + tbl = exp.to_table(table) + cols = [exp.column(c, quoted=True) for c in key_cols] + predicate = None + for col in cols: + term = exp.Not(this=exp.Is(this=col.copy(), expression=exp.null())) + predicate = term if predicate is None else exp.and_(predicate, term) + + count_star = exp.func("COUNT", exp.Star()).as_("c") + rows_q = exp.select(count_star).from_(tbl.copy()).where(predicate) + inner = ( + exp.select(*[c.copy() for c in cols]) + .from_(tbl.copy()) + .where(predicate.copy()) + .distinct() + ) + dist_q = exp.select(count_star.copy()).from_(inner.subquery("d")) + return ( + rows_q.sql(dialect=sqlglot_name, identify=True), + dist_q.sql(dialect=sqlglot_name, identify=True), + ) + + async def _side_stats( + self, *, client, table, key_cols, sqlglot_name, datasource, + ) -> SideStats: + """Full-scan one side of a join: non-null key rows vs distinct key-tuples.""" + rows_sql, dist_sql = self._side_stats_sql( + table=table, key_cols=key_cols, sqlglot_name=sqlglot_name, + ) + # Give the correlated-subquery guard a version to gate on. + await self._preflight_clickhouse_correlated( + dialect=sqlglot_name, datasource=datasource + ) + # Profile the tenant-scoped rows this session may see, like every + # other execution path; a no-op when no SessionPolicy is configured. + rows_sql = self._apply_policy( + sql=rows_sql, dialect=sqlglot_name, datasource=datasource + ) + dist_sql = self._apply_policy( + sql=dist_sql, dialect=sqlglot_name, datasource=datasource + ) + row_rows = await client.execute(sql=rows_sql) + dist_rows = await client.execute(sql=dist_sql) + row_count = int(next(iter(row_rows[0].values()))) + distinct_count = int(next(iter(dist_rows[0].values()))) + return SideStats( + row_count=row_count, + distinct_count=distinct_count, + observed_unique=(row_count == distinct_count), + ) + + async def _persist_join_cardinality( + self, *, data_source, model_name, items, + ) -> None: + m = await self.storage.get_model(model_name, data_source=data_source) + if m is None: + return + sig_to_detected = dict(items) + changed = False + new_joins = [] + for j in m.joins: + d = sig_to_detected.get(_join_signature(j)) + if d is not None and j.cardinality != d: + new_joins.append(j.model_copy(update={"cardinality": d})) + changed = True + else: + new_joins.append(j) + if changed: + await self.storage.save_model(m.model_copy(update={"joins": new_joins})) + async def validate_models( self, data_source: str | None = None ) -> "list[Any]": @@ -4067,3 +4324,72 @@ def _dialect_for_type(ds_type: str | None) -> str: Unknown / ``None`` / empty ds-types fall back to ``"postgres"``. """ return dialect_for_ds_type(ds_type).sqlglot_name + + +# --------------------------------------------------------------------------- +# Join-cardinality detection helpers +# --------------------------------------------------------------------------- + + +def _join_signature(join) -> tuple: + """Stable identity for a join: (target_model, sorted key pairs).""" + return (join.target_model, tuple(sorted((p[0], p[1]) for p in join.join_pairs))) + + +def _scan_failed_finding(*, data_source, model, join, exc) -> JoinCardinalityFinding: + """Report a join whose profiling scan raised, instead of aborting.""" + logger.warning( + "detect_join_cardinality: %s -> %s failed: %s", + model.name, join.target_model, exc, + ) + return JoinCardinalityFinding( + data_source=data_source, + model=model.name, + target_model=join.target_model, + join_pairs=[[p[0], p[1]] for p in join.join_pairs], + stored=join.cardinality, + detected=None, + verdict=CardinalityVerdict.SCAN_FAILED, + note=f"profiling scan failed: {exc}", + ) + + +def _detection_skip_reason(*, model, target, src_cols, tgt_cols) -> str | None: + """Why a join can't be profiled; ``None`` when it can.""" + if model.sql_table is None: + return ( + f"model {model.name!r} is not table-backed (sql/query-backed); " + f"cardinality profiling supports sql_table models only" + ) + if target is None or target.sql_table is None: + tn = target.name if target is not None else "?" + return f"join target {tn!r} is not a table-backed model; skipped" + for mdl, cols in ((model, src_cols), (target, tgt_cols)): + for c in cols: + col = next((x for x in mdl.columns if x.name == c), None) + if col is not None and col.sql is not None and col.sql.strip() != c: + return ( + f"join key {mdl.name}.{c!r} is a SQL expression; " + f"cardinality profiling supports bare-column keys only" + ) + return None + + +def _unique_contradictions( + *, model, target, src_cols, tgt_cols, src_side, tgt_side, +) -> list[str]: + """Single-column join keys declared unique/PK but observed to have dups.""" + out: list[str] = [] + for mdl, cols, side in ( + (model, src_cols, src_side), + (target, tgt_cols, tgt_side), + ): + if len(cols) != 1 or side.observed_unique: + continue + c = cols[0] + col = next((x for x in mdl.columns if x.name == c), None) + if col is not None and declares_solo_unique(columns=mdl.columns, column=col): + out.append( + f"{mdl.name}.{c} is declared unique but the data has duplicates" + ) + return out diff --git a/slayer/facade/catalog.py b/slayer/facade/catalog.py index 86563141..9542e1a0 100644 --- a/slayer/facade/catalog.py +++ b/slayer/facade/catalog.py @@ -22,6 +22,7 @@ DEFAULT_AGGREGATIONS_BY_TYPE, PRIMARY_KEY_AGGREGATIONS, DataType, + JoinCardinality, JoinType, ) from slayer.core.models import ( @@ -75,6 +76,7 @@ class FacadeJoin(BaseModel): target_model: str join_pairs: list[list[str]] join_type: JoinType = JoinType.LEFT + cardinality: JoinCardinality | None = None class FacadeTable(BaseModel): @@ -344,6 +346,7 @@ def _facade_join_from(*, join: ModelJoin) -> FacadeJoin: target_model=join.target_model, join_pairs=[list(pair) for pair in join.join_pairs], join_type=join.join_type, + cardinality=join.cardinality, ) diff --git a/slayer/facade/translator.py b/slayer/facade/translator.py index 8c16d70c..eb2582e9 100644 --- a/slayer/facade/translator.py +++ b/slayer/facade/translator.py @@ -44,7 +44,7 @@ import sqlglot.expressions as exp from pydantic import BaseModel, ConfigDict -from slayer.core.enums import DataType, JoinType, TimeGranularity +from slayer.core.enums import DataType, JoinCardinality, JoinType, TimeGranularity from slayer.core.models import ModelJoin, SlayerModel from slayer.core.query import ( ColumnRef, @@ -53,6 +53,7 @@ SlayerQuery, TimeDimension, ) +from slayer.engine.cardinality import declares_solo_unique from slayer.facade.catalog import ( CATALOG_NAME, FacadeCatalog, @@ -2920,10 +2921,26 @@ def _build_source_model_from_join( target_model=plan.target_table.name, join_pairs=[[plan.source_col, plan.target_col]], join_type=JoinType.LEFT, + cardinality=_dynamic_join_cardinality(plan), )], ) +def _dynamic_join_cardinality(plan: "_JoinPlan") -> JoinCardinality | None: + """``many_to_one`` when the target column alone is unique, else undetermined.""" + model_ref = plan.target_table.model_ref + if model_ref is None: + return None + for col in model_ref.columns: + if col.name.lower() == plan.target_col.lower(): + # The join constrains one column, so a composite-PK member does + # not qualify — same subset rule as is_key_set_unique. + if declares_solo_unique(columns=model_ref.columns, column=col): + return JoinCardinality.MANY_TO_ONE + return None + return None + + def _emit_join_warnings(plan: _JoinPlan, parent_name: str) -> None: if plan.is_dynamic: logger.warning( diff --git a/slayer/inspect/model_render.py b/slayer/inspect/model_render.py index 27f100f5..2aca5e04 100644 --- a/slayer/inspect/model_render.py +++ b/slayer/inspect/model_render.py @@ -935,6 +935,7 @@ async def _persist_sample( "name": c.name, "type": _render_column_type(c), "primary_key": "yes" if c.primary_key else "", + "unique": "yes" if c.unique else "", "sql": c.sql if c.sql else c.name, "allowed_aggregations": aggs, "filter": c.filter, @@ -944,7 +945,7 @@ async def _persist_sample( "sampled": sampled_cell, }) col_columns = [ - "name", "type", "primary_key", "sql", "allowed_aggregations", + "name", "type", "primary_key", "unique", "sql", "allowed_aggregations", "filter", "label", "description", "meta", "sampled", ] if not show_sql: @@ -1031,12 +1032,13 @@ async def _persist_sample( join_rows.append({ "target_model": j.target_model, "join_pairs": pairs, + "cardinality": str(j.cardinality) if j.cardinality else "", }) out_sections.append( f"## Joins ({len(join_rows)})\n\n" + _markdown_table( rows=join_rows, - columns=["target_model", "join_pairs"], + columns=["target_model", "join_pairs", "cardinality"], ) ) elif model.joins: @@ -1218,6 +1220,7 @@ async def _persist_sample( if c.type.is_opaque else {} ), "primary_key": c.primary_key, + "unique": c.unique, **({"sql": c.sql} if show_sql else {}), "allowed_aggregations": c.allowed_aggregations, **({"filter": c.filter} if show_sql else {}), @@ -1280,6 +1283,7 @@ async def _persist_sample( { "target_model": j.target_model, "join_pairs": j.join_pairs, + "cardinality": j.cardinality, } for j in model.joins ] diff --git a/slayer/mcp/server.py b/slayer/mcp/server.py index 02ad87e1..fe94ae48 100644 --- a/slayer/mcp/server.py +++ b/slayer/mcp/server.py @@ -935,8 +935,10 @@ async def create_model( description: What this model represents. columns: List of column definitions. Each: {"name": "col", "sql": "col", "type": "string"}. Types: string, number, time, date, boolean. Optional fields: ``primary_key``, - ``allowed_aggregations`` (whitelist), ``filter`` (CASE WHEN inside aggregation), - ``label``, ``description``, ``hidden``, ``meta``. + ``unique`` (single-column uniqueness that is not the PK; ``primary_key`` + already implies it), ``allowed_aggregations`` (whitelist), ``filter`` + (CASE WHEN inside aggregation), ``label``, ``description``, ``hidden``, + ``meta``. measures: List of named formula definitions on the model. Each: {"name": "aov", "formula": "revenue:sum / *:count", "label": "...", "description": "...", "meta": {...}}. @@ -1092,10 +1094,14 @@ async def edit_model( meta: Arbitrary JSON metadata for the model (replaces existing meta). Pass null/None to clear. columns: Columns to create or update (upsert by name). Each dict: {"name": "col", "type": "string", "sql": "col", "description": "...", - "primary_key": false, "hidden": false, "allowed_aggregations": ["sum", "avg"], + "primary_key": false, "unique": false, "hidden": false, + "allowed_aggregations": ["sum", "avg"], "filter": "status = 'active'", "label": "..."}. If a column with this name exists, only the provided fields are updated. Types: string, number, time, date, boolean. + ``unique`` marks single-column uniqueness that is not the primary key + (``primary_key`` already implies it); it is used to infer join + cardinality. measures: Named formula measures to create or update (upsert by name). Each dict: {"name": "aov", "formula": "revenue:sum / *:count", "label": "...", "description": "...", "meta": {...}}. @@ -1107,7 +1113,14 @@ async def edit_model( "meta": {...}}. ``meta`` is an optional opaque dict for caller bookkeeping. joins: Joins to create or update (upsert by target_model). Each dict: - {"target_model": "customers", "join_pairs": [["customer_id", "id"]]}. + {"target_model": "customers", "join_pairs": [["customer_id", "id"]], + "cardinality": "many_to_one", "description": "...", "meta": {...}}. + A composite key is one join with several ``join_pairs`` entries, not + one join per column. ``cardinality`` is the join's arity read + source->target, one of ``one_to_one`` / ``one_to_many`` / + ``many_to_one`` / ``many_to_many``; omit it when undetermined. It is + descriptive metadata only — it changes neither ``join_type`` nor + query results. add_filters: SQL filter strings to add (e.g. ["deleted_at IS NULL"]). Duplicates ignored. remove_filters: SQL filter strings to remove (exact match). remove: Named entities to delete, keyed by type: diff --git a/slayer/osi/converter.py b/slayer/osi/converter.py index a22f5940..e7a3fcad 100644 --- a/slayer/osi/converter.py +++ b/slayer/osi/converter.py @@ -18,7 +18,7 @@ import sqlglot import sqlglot.expressions as exp -from slayer.core.enums import DataType +from slayer.core.enums import DataType, JoinCardinality from slayer.core.formula import parse_formula from slayer.core.models import Column, ModelJoin, ModelMeasure, SlayerModel from slayer.core.refs import IDENTIFIER_RE as _IDENTIFIER_RE @@ -525,6 +525,8 @@ def _build_join(self, rel: OSIRelationship) -> None: self._models[src].joins.append(ModelJoin( target_model=rel.to, join_pairs=pairs, + # OSI relationships are direction-implied: from = many, to = one. + cardinality=JoinCardinality.MANY_TO_ONE, description=_render_description(explicit=None, ctx=rel.ai_context), meta=_build_meta(ctx=rel.ai_context, custom_extensions=rel.custom_extensions), )) diff --git a/slayer/search/graph.py b/slayer/search/graph.py index 61880c55..f1d7848b 100644 --- a/slayer/search/graph.py +++ b/slayer/search/graph.py @@ -181,7 +181,7 @@ def _create_schema(conn: Any) -> None: "FROM Model TO Aggregation" ")" ) - conn.execute("CREATE REL TABLE JOINS(FROM Model TO Model)") + conn.execute("CREATE REL TABLE JOINS(FROM Model TO Model, cardinality STRING)") def _insert_model_child_nodes(conn: Any, canonical_model: str, model: Any) -> None: @@ -320,8 +320,12 @@ def _insert_joins_edges(conn: Any, visible_models: dict) -> None: continue conn.execute( "MATCH (src:Model {id: $src}), (tgt:Model {id: $tgt}) " - "CREATE (src)-[:JOINS]->(tgt)", - {"src": canonical_model, "tgt": target_canonical}, + "CREATE (src)-[:JOINS {cardinality: $card}]->(tgt)", + { + "src": canonical_model, + "tgt": target_canonical, + "card": str(join.cardinality) if join.cardinality else "", + }, ) diff --git a/slayer/storage/join_sync.py b/slayer/storage/join_sync.py index aa6bf377..0609af02 100644 --- a/slayer/storage/join_sync.py +++ b/slayer/storage/join_sync.py @@ -13,7 +13,7 @@ """ -from slayer.core.enums import JoinType +from slayer.core.enums import JoinType, invert_cardinality from slayer.core.models import DatasourceConfig, ModelJoin, SlayerModel from slayer.embeddings.models import Embedding from slayer.memories.models import Memory @@ -38,20 +38,24 @@ async def _mirror_inner_joins(model: SlayerModel, storage: StorageBackend) -> No if target is None: continue reverse_pairs = [[tgt, src] for src, tgt in join.join_pairs] + reverse_cardinality = invert_cardinality(join.cardinality) existing = next( (j for j in target.joins if j.target_model == model.name and j.join_type == JoinType.INNER), None, ) if existing is not None: - if existing.join_pairs != reverse_pairs: + if (existing.join_pairs != reverse_pairs + or existing.cardinality != reverse_cardinality): existing.join_pairs = reverse_pairs + existing.cardinality = reverse_cardinality await storage.save_model(target) else: target.joins.append(ModelJoin( target_model=model.name, join_pairs=reverse_pairs, join_type=JoinType.INNER, + cardinality=reverse_cardinality, )) await storage.save_model(target) diff --git a/tests/test_cardinality_helpers.py b/tests/test_cardinality_helpers.py new file mode 100644 index 00000000..2ef27a1a --- /dev/null +++ b/tests/test_cardinality_helpers.py @@ -0,0 +1,161 @@ +"""Pure, DB-free cardinality-inference helpers.""" + +from slayer.core.enums import DataType, JoinCardinality +from slayer.core.models import Column +from slayer.engine.cardinality import ( + classify_cardinality, + declares_solo_unique, + infer_structural_cardinality, + is_key_set_unique, +) + + +# --------------------------------------------------------------------------- +# is_key_set_unique — subset, not exact/superset +# --------------------------------------------------------------------------- + + +def test_single_column_exact_match_is_unique() -> None: + assert is_key_set_unique(key_columns=["id"], unique_key_sets=[["id"]]) is True + + +def test_composite_key_with_subset_constraint_is_unique() -> None: + # (org_id) unique => (org_id, code) unique. + assert ( + is_key_set_unique( + key_columns=["org_id", "code"], unique_key_sets=[["org_id"]] + ) + is True + ) + + +def test_composite_constraint_exactly_covering_is_unique() -> None: + assert ( + is_key_set_unique( + key_columns=["org_id", "code"], unique_key_sets=[["org_id", "code"]] + ) + is True + ) + + +def test_superset_constraint_does_not_imply_unique() -> None: + # A unique constraint on (org_id, code) does NOT make (org_id) alone unique. + assert ( + is_key_set_unique(key_columns=["org_id"], unique_key_sets=[["org_id", "code"]]) + is False + ) + + +def test_unrelated_constraint_is_not_unique() -> None: + assert is_key_set_unique(key_columns=["a", "b"], unique_key_sets=[["c"]]) is False + + +def test_no_constraints_is_not_unique() -> None: + assert is_key_set_unique(key_columns=["a"], unique_key_sets=[]) is False + + +def test_subset_match_is_order_independent() -> None: + assert ( + is_key_set_unique( + key_columns=["code", "org_id"], unique_key_sets=[["org_id"]] + ) + is True + ) + + +# --------------------------------------------------------------------------- +# classify_cardinality — total, data-profiling classification +# --------------------------------------------------------------------------- + + +def test_classify_both_unique_is_one_to_one() -> None: + assert ( + classify_cardinality(source_unique=True, target_unique=True) + is JoinCardinality.ONE_TO_ONE + ) + + +def test_classify_target_unique_source_not_is_many_to_one() -> None: + assert ( + classify_cardinality(source_unique=False, target_unique=True) + is JoinCardinality.MANY_TO_ONE + ) + + +def test_classify_source_unique_target_not_is_one_to_many() -> None: + assert ( + classify_cardinality(source_unique=True, target_unique=False) + is JoinCardinality.ONE_TO_MANY + ) + + +def test_classify_neither_unique_is_many_to_many() -> None: + assert ( + classify_cardinality(source_unique=False, target_unique=False) + is JoinCardinality.MANY_TO_MANY + ) + + +# --------------------------------------------------------------------------- +# infer_structural_cardinality — honest ingest-time guess +# --------------------------------------------------------------------------- + + +def test_structural_target_verified_unique_source_not_is_many_to_one() -> None: + assert ( + infer_structural_cardinality(source_unique=False, target_verified_unique=True) + is JoinCardinality.MANY_TO_ONE + ) + + +def test_structural_both_unique_is_one_to_one() -> None: + assert ( + infer_structural_cardinality(source_unique=True, target_verified_unique=True) + is JoinCardinality.ONE_TO_ONE + ) + + +def test_structural_target_not_verified_returns_none() -> None: + # Declared relationship with no known PK/unique on the target: undetermined. + assert ( + infer_structural_cardinality(source_unique=True, target_verified_unique=False) + is None + ) + assert ( + infer_structural_cardinality(source_unique=False, target_verified_unique=False) + is None + ) + + +# --------------------------------------------------------------------------- +# declares_solo_unique — a composite-PK member claims nothing on its own +# --------------------------------------------------------------------------- + + +def _c(name: str, *, pk: bool = False, unique: bool = False) -> Column: + return Column(name=name, type=DataType.INT, primary_key=pk, unique=unique) + + +def test_solo_pk_column_declares_uniqueness() -> None: + cols = [_c("id", pk=True), _c("amount")] + assert declares_solo_unique(columns=cols, column=cols[0]) is True + + +def test_composite_pk_member_does_not_declare_uniqueness() -> None: + # PK (id, sku): every member is stamped primary_key, but neither is unique + # alone — the same subset rule is_key_set_unique applies. + cols = [_c("id", pk=True), _c("sku", pk=True), _c("cost")] + assert declares_solo_unique(columns=cols, column=cols[0]) is False + assert declares_solo_unique(columns=cols, column=cols[1]) is False + + +def test_explicit_unique_flag_declares_uniqueness_even_with_composite_pk() -> None: + # `unique` is single-column by definition, so it stands on its own + # regardless of how many PK columns the model has. + cols = [_c("id", pk=True), _c("sku", pk=True), _c("email", unique=True)] + assert declares_solo_unique(columns=cols, column=cols[2]) is True + + +def test_plain_column_declares_nothing() -> None: + cols = [_c("id", pk=True), _c("amount")] + assert declares_solo_unique(columns=cols, column=cols[1]) is False diff --git a/tests/test_cli_validate_models_cardinality.py b/tests/test_cli_validate_models_cardinality.py new file mode 100644 index 00000000..44755b1f --- /dev/null +++ b/tests/test_cli_validate_models_cardinality.py @@ -0,0 +1,507 @@ +"""CLI ``slayer validate-models``, including the merged cardinality profiling.""" + +from __future__ import annotations + +import json +import sqlite3 +import sys +import tempfile +from contextlib import contextmanager +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from slayer.async_utils import run_sync +from slayer.cli import _run_validate_models +from slayer.cli import main as cli_main +from slayer.core.enums import DataType +from slayer.core.models import Column, DatasourceConfig, ModelJoin, SlayerModel +from slayer.engine.query_engine import SlayerQueryEngine +from slayer.storage.yaml_storage import YAMLStorage + + +@pytest.fixture +def workspace(): + tmp = tempfile.TemporaryDirectory() + try: + yield Path(tmp.name) + finally: + tmp.cleanup() + + +# --------------------------------------------------------------------------- +# Fixture: one datasource, two models, drift optionally seeded on each +# --------------------------------------------------------------------------- + + +def _seed_db(db_path: str) -> None: + conn = sqlite3.connect(db_path) + conn.executescript( + """ + CREATE TABLE customers (id INTEGER PRIMARY KEY, region TEXT); + CREATE TABLE orders (id INTEGER PRIMARY KEY, customer_id INTEGER); + INSERT INTO customers VALUES (1,'US'),(2,'EU'); + INSERT INTO orders VALUES (1,1),(2,1),(3,2); + """ + ) + conn.commit() + conn.close() + + +def _customers_model(*, data_source: str, drift: bool) -> SlayerModel: + columns = [ + Column(name="id", sql="id", type=DataType.INT, primary_key=True), + Column(name="region", sql="region", type=DataType.TEXT), + ] + if drift: + # Not in the live table -> validate_models emits an EditModelDelete. + columns.append(Column(name="ghost_c", sql="ghost_c", type=DataType.TEXT)) + return SlayerModel( + name="customers", sql_table="customers", + data_source=data_source, columns=columns, + ) + + +def _orders_model(*, data_source: str, drift: bool) -> SlayerModel: + columns = [ + Column(name="id", sql="id", type=DataType.INT, primary_key=True), + Column(name="customer_id", sql="customer_id", type=DataType.INT), + ] + if drift: + columns.append(Column(name="ghost_o", sql="ghost_o", type=DataType.TEXT)) + return SlayerModel( + name="orders", sql_table="orders", + data_source=data_source, columns=columns, + joins=[ModelJoin(target_model="customers", join_pairs=[["customer_id", "id"]])], + ) + + +def _setup( + workspace: Path, + *, + ds_name: str = "ds", + drift_orders: bool = False, + drift_customers: bool = False, + store: str | None = None, + db_name: str | None = None, +) -> str: + db = str(workspace / (db_name or f"{ds_name}.db")) + _seed_db(db) + store = store or str(workspace / "store") + storage = YAMLStorage(base_dir=store) + run_sync(storage.save_datasource( + DatasourceConfig(name=ds_name, type="sqlite", database=db) + )) + run_sync(storage.save_model( + _customers_model(data_source=ds_name, drift=drift_customers) + )) + run_sync(storage.save_model( + _orders_model(data_source=ds_name, drift=drift_orders) + )) + return store + + +def _args(store: str, **kw) -> SimpleNamespace: + base = dict( + datasource="ds", + model=None, + cardinality=False, + persist_cardinality=False, + format="text", + force_clean=False, + yes=False, + storage=store, + models_dir=None, + ) + base.update(kw) + return SimpleNamespace(**base) + + +@contextmanager +def _argv(*argv: str): + original = sys.argv + sys.argv = ["slayer", *argv] + try: + yield + finally: + sys.argv = original + + +def _exit_code(*argv: str) -> int: + with _argv(*argv): + try: + cli_main() + except SystemExit as exc: # NOSONAR(S5754) — capturing the CLI exit code + return int(exc.code or 0) + return 0 + + +# --------------------------------------------------------------------------- +# Default output is unchanged (no cardinality work at all) +# --------------------------------------------------------------------------- + + +def test_default_output_has_no_section_headers(workspace: Path, capsys) -> None: + store = _setup(workspace) + _run_validate_models(_args(store)) + out = capsys.readouterr().out + assert "Join cardinality" not in out + assert "Schema drift" not in out + assert "No drift detected." in out + + +def test_default_run_never_profiles(workspace: Path, monkeypatch, capsys) -> None: + store = _setup(workspace) + calls: list[str] = [] + + async def _spy(self, **kwargs): + calls.append("called") + raise AssertionError("cardinality profiling must not run by default") + + monkeypatch.setattr(SlayerQueryEngine, "detect_join_cardinality", _spy) + _run_validate_models(_args(store)) + capsys.readouterr() + assert calls == [] + + +# --------------------------------------------------------------------------- +# --cardinality +# --------------------------------------------------------------------------- + + +def test_cardinality_text_output_has_both_sections(workspace: Path, capsys) -> None: + store = _setup(workspace) + _run_validate_models(_args(store, cardinality=True)) + out = capsys.readouterr().out + assert "Schema drift" in out + assert "Join cardinality" in out + assert "many_to_one" in out.split("Join cardinality", 1)[1] + + +def test_cardinality_leaves_storage_untouched(workspace: Path, capsys) -> None: + store = _setup(workspace) + _run_validate_models(_args(store, cardinality=True)) + capsys.readouterr() + orders = run_sync(YAMLStorage(base_dir=store).get_model("orders", data_source="ds")) + assert orders.joins[0].cardinality is None + + +def test_persist_cardinality_implies_cardinality(workspace: Path, capsys) -> None: + store = _setup(workspace) + _run_validate_models(_args(store, persist_cardinality=True)) + out = capsys.readouterr().out + assert "Join cardinality" in out + orders = run_sync(YAMLStorage(base_dir=store).get_model("orders", data_source="ds")) + assert orders.joins[0].cardinality is not None + + +# --------------------------------------------------------------------------- +# --format json +# --------------------------------------------------------------------------- + + +def test_json_with_cardinality(workspace: Path, capsys) -> None: + store = _setup(workspace) + _run_validate_models(_args(store, cardinality=True, format="json")) + data = json.loads(capsys.readouterr().out) + assert isinstance(data["drift"], list) + finding = next(f for f in data["cardinality"]["findings"] if f["model"] == "orders") + assert finding["detected"] == "many_to_one" + + +def test_json_without_cardinality_is_null(workspace: Path, capsys) -> None: + store = _setup(workspace) + _run_validate_models(_args(store, format="json")) + data = json.loads(capsys.readouterr().out) + assert data["cardinality"] is None + assert data["drift"] == [] + + +def test_json_serialises_drift_entries(workspace: Path, capsys) -> None: + """Both ToDeleteEntry variants must round-trip through json.dumps.""" + store = _setup(workspace, drift_orders=True, drift_customers=True) + # A model whose whole table is gone -> WholeModelDelete. + run_sync(YAMLStorage(base_dir=store).save_model(SlayerModel( + name="vanished", sql_table="vanished", data_source="ds", + columns=[Column(name="id", sql="id", type=DataType.INT)], + ))) + _run_validate_models(_args(store, format="json")) + data = json.loads(capsys.readouterr().out) + tools = {e["tool"] for e in data["drift"]} + assert "edit_model" in tools + assert "delete_model" in tools + edit = next(e for e in data["drift"] if e["tool"] == "edit_model") + assert "ghost_o" in edit["remove"]["columns"] or "ghost_c" in edit["remove"]["columns"] + assert isinstance(edit["reasons"], list) + + +# --------------------------------------------------------------------------- +# --model scoping +# --------------------------------------------------------------------------- + + +def test_model_scopes_both_sections(workspace: Path, capsys) -> None: + store = _setup(workspace, drift_orders=True, drift_customers=True) + _run_validate_models(_args(store, model="orders", cardinality=True, format="json")) + data = json.loads(capsys.readouterr().out) + assert {e["model_name"] for e in data["drift"]} == {"orders"} + assert {f["model"] for f in data["cardinality"]["findings"]} == {"orders"} + + +def test_model_scopes_force_clean_mutations(workspace: Path, capsys) -> None: + store = _setup(workspace, drift_orders=True, drift_customers=True) + _run_validate_models(_args(store, model="orders", force_clean=True, yes=True)) + capsys.readouterr() + storage = YAMLStorage(base_dir=store) + orders = run_sync(storage.get_model("orders", data_source="ds")) + customers = run_sync(storage.get_model("customers", data_source="ds")) + assert [c.name for c in orders.columns] == ["id", "customer_id"] + assert "ghost_c" in [c.name for c in customers.columns] + + +def test_force_clean_ignores_out_of_scope_residual(workspace: Path, capsys) -> None: + """Drift on a model outside --model must not print, nor force exit 1.""" + store = _setup(workspace, drift_orders=True, drift_customers=True) + _run_validate_models(_args(store, model="orders", force_clean=True, yes=True)) + out = capsys.readouterr().out + assert "ghost_c" not in out + + +def test_unknown_model_fails_fast(workspace: Path, capsys) -> None: + store = _setup(workspace) + args = _args(store, model="nope") + with pytest.raises(SystemExit) as exc: + _run_validate_models(args) + assert exc.value.code == 1 + assert "nope" in capsys.readouterr().err + + +def test_unknown_datasource_fails_fast(workspace: Path, capsys) -> None: + store = _setup(workspace) + args = _args(store, datasource="nope") + with pytest.raises(SystemExit) as exc: + _run_validate_models(args) + assert exc.value.code == 1 + assert "nope" in capsys.readouterr().err + + +def test_model_resolves_across_datasources(workspace: Path, capsys) -> None: + store = _setup(workspace, ds_name="ds") + _setup(workspace, ds_name="ds2", store=store) + _run_validate_models( + _args(store, datasource=None, model="orders", cardinality=True, format="json") + ) + data = json.loads(capsys.readouterr().out) + sources = {f["data_source"] for f in data["cardinality"]["findings"]} + assert sources == {"ds", "ds2"} + + +def test_model_present_in_only_one_datasource_does_not_fail( + workspace: Path, capsys +) -> None: + store = _setup(workspace, ds_name="ds") + _setup(workspace, ds_name="ds2", store=store) + storage = YAMLStorage(base_dir=store) + run_sync(storage.save_model(SlayerModel( + name="solo", sql_table="customers", data_source="ds2", + columns=[Column(name="id", sql="id", type=DataType.INT, primary_key=True)], + ))) + _run_validate_models( + _args(store, datasource=None, model="solo", cardinality=True, format="json") + ) + data = json.loads(capsys.readouterr().out) + assert data["cardinality"]["findings"] == [] + + +# --------------------------------------------------------------------------- +# Exit codes and failure handling +# --------------------------------------------------------------------------- + + +def test_contradicts_hard_still_exits_zero(workspace: Path, capsys) -> None: + from slayer.core.enums import JoinCardinality + + store = _setup(workspace) + storage = YAMLStorage(base_dir=store) + orders = run_sync(storage.get_model("orders", data_source="ds")) + # customer_id has duplicates, so a stored one_to_many is hard-contradicted. + orders.joins[0].cardinality = JoinCardinality.ONE_TO_MANY + run_sync(storage.save_model(orders)) + + _run_validate_models(_args(store, cardinality=True, format="json")) + data = json.loads(capsys.readouterr().out) + finding = next(f for f in data["cardinality"]["findings"] if f["model"] == "orders") + assert finding["verdict"] == "contradicts_hard" + + +def test_datasource_failure_in_unscoped_run_exits_one( + workspace: Path, monkeypatch, capsys +) -> None: + store = _setup(workspace, ds_name="ds") + _setup(workspace, ds_name="ds2", store=store) + + async def _flaky(self, data_source=None): + if data_source == "ds2": + raise RuntimeError("boom") + return [] + + monkeypatch.setattr(SlayerQueryEngine, "validate_models", _flaky) + + async def _never(self, **kwargs): + raise AssertionError("must not profile after a datasource failure") + + monkeypatch.setattr(SlayerQueryEngine, "detect_join_cardinality", _never) + + args = _args(store, datasource=None, cardinality=True) + with pytest.raises(SystemExit) as exc: + _run_validate_models(args) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert "ds2" in err + assert "boom" in err + + +def test_every_datasource_failure_is_reported( + workspace: Path, monkeypatch, capsys +) -> None: + """The per-datasource loop must not short-circuit on the first failure.""" + store = _setup(workspace, ds_name="ds") + _setup(workspace, ds_name="ds2", store=store) + + async def _boom(self, data_source=None): + raise RuntimeError(f"{data_source} exploded") + + monkeypatch.setattr(SlayerQueryEngine, "validate_models", _boom) + + args = _args(store, datasource=None) + with pytest.raises(SystemExit): + _run_validate_models(args) + err = capsys.readouterr().err + assert "ds exploded" in err + assert "ds2 exploded" in err + + +def test_datasource_failure_keeps_json_parseable( + workspace: Path, monkeypatch, capsys +) -> None: + store = _setup(workspace, ds_name="ds") + _setup(workspace, ds_name="ds2", store=store) + + async def _flaky(self, data_source=None): + if data_source == "ds2": + raise RuntimeError("boom") + return [] + + monkeypatch.setattr(SlayerQueryEngine, "validate_models", _flaky) + + args = _args(store, datasource=None, format="json") + with pytest.raises(SystemExit): + _run_validate_models(args) + captured = capsys.readouterr() + assert json.loads(captured.out) == {"drift": [], "cardinality": None} + assert "ds2" in captured.err + + +def test_drift_failure_skips_profiling(workspace: Path, monkeypatch, capsys) -> None: + store = _setup(workspace) + + async def _boom(self, data_source=None): + raise RuntimeError("drift exploded") + + monkeypatch.setattr(SlayerQueryEngine, "validate_models", _boom) + + async def _never(self, **kwargs): + raise AssertionError("must not profile when drift validation failed") + + monkeypatch.setattr(SlayerQueryEngine, "detect_join_cardinality", _never) + + args = _args(store, cardinality=True) + with pytest.raises(SystemExit) as exc: + _run_validate_models(args) + assert exc.value.code == 1 + assert "drift exploded" in capsys.readouterr().err + + +def test_cardinality_failure_exits_one(workspace: Path, monkeypatch, capsys) -> None: + store = _setup(workspace) + + async def _boom(self, **kwargs): + raise RuntimeError("profiling exploded") + + monkeypatch.setattr(SlayerQueryEngine, "detect_join_cardinality", _boom) + + args = _args(store, cardinality=True) + with pytest.raises(SystemExit) as exc: + _run_validate_models(args) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert "cardinality" in err.lower() + assert "profiling exploded" in err + + +def test_scan_failure_reports_fully_but_exits_one( + workspace: Path, monkeypatch, capsys +) -> None: + """A contained scan failure is still work the command did not do.""" + store = _setup(workspace) + + async def _boom(self, **kwargs): + raise RuntimeError("table is on fire") + + monkeypatch.setattr(SlayerQueryEngine, "_side_stats", _boom) + + args = _args(store, cardinality=True) + with pytest.raises(SystemExit) as exc: + _run_validate_models(args) + assert exc.value.code == 1 + captured = capsys.readouterr() + # The report is still printed in full — containment is not silence. + assert "scan_failed" in captured.out + assert "could not be profiled" in captured.err + + +def test_profiling_runs_after_the_force_clean_apply( + workspace: Path, monkeypatch, capsys +) -> None: + store = _setup(workspace, drift_orders=True) + order: list[str] = [] + + real_apply = SlayerQueryEngine.apply_drift_deletes + real_detect = SlayerQueryEngine.detect_join_cardinality + + async def _apply(self, deletes): + order.append("apply") + return await real_apply(self, deletes) + + async def _detect(self, **kwargs): + order.append("detect") + return await real_detect(self, **kwargs) + + monkeypatch.setattr(SlayerQueryEngine, "apply_drift_deletes", _apply) + monkeypatch.setattr(SlayerQueryEngine, "detect_join_cardinality", _detect) + + _run_validate_models( + _args(store, cardinality=True, force_clean=True, yes=True) + ) + capsys.readouterr() + assert order == ["apply", "detect"] + + +# --------------------------------------------------------------------------- +# Parser-level contracts +# --------------------------------------------------------------------------- + + +def test_json_with_force_clean_is_a_usage_error(workspace: Path) -> None: + store = _setup(workspace) + assert _exit_code( + "validate-models", "--storage", store, "--format", "json", "--force-clean" + ) == 2 + + +def test_joins_command_is_gone(workspace: Path) -> None: + store = _setup(workspace) + assert _exit_code( + "joins", "detect-cardinality", "--storage", store + ) == 2 diff --git a/tests/test_detect_join_cardinality.py b/tests/test_detect_join_cardinality.py new file mode 100644 index 00000000..8b11065e --- /dev/null +++ b/tests/test_detect_join_cardinality.py @@ -0,0 +1,666 @@ +"""Opt-in data-profiling cardinality detection via ``engine.detect_join_cardinality``.""" + +from __future__ import annotations + +import sqlite3 +import tempfile +from pathlib import Path +from unittest.mock import patch + +import pytest + +from slayer.core.enums import DataType, JoinCardinality +from slayer.core.errors import ForcedFilterError +from slayer.core.models import Column, DatasourceConfig, ModelJoin, SlayerModel +from slayer.core.policy import ColumnFilterRuleset, SessionPolicy +from slayer.engine.cardinality import CardinalityVerdict, JoinCardinalityReport +from slayer.engine.query_engine import SlayerQueryEngine +from slayer.sql.client import SlayerSQLClient +from slayer.storage.yaml_storage import YAMLStorage + + +@pytest.fixture +def workspace(): + tmp = tempfile.TemporaryDirectory() + try: + yield Path(tmp.name) + finally: + tmp.cleanup() + + +def _seed_db(db_path: str) -> None: + conn = sqlite3.connect(db_path) + conn.executescript( + """ + CREATE TABLE customers (id INTEGER PRIMARY KEY, region TEXT); + CREATE TABLE orders (id INTEGER PRIMARY KEY, customer_id INTEGER); + CREATE TABLE user_profiles (customer_id INTEGER PRIMARY KEY, bio TEXT); + CREATE TABLE carts (id INTEGER PRIMARY KEY); + CREATE TABLE cart_lines (id INTEGER PRIMARY KEY, cart_id INTEGER); + CREATE TABLE left_tbl (k INTEGER); + CREATE TABLE right_tbl (k INTEGER, label TEXT); + CREATE TABLE ck_parent (a INTEGER, b TEXT, PRIMARY KEY (a, b)); + CREATE TABLE ck_child (a INTEGER, b TEXT); + CREATE TABLE empty_src (k INTEGER); + CREATE TABLE empty_tgt (k INTEGER PRIMARY KEY); + CREATE TABLE all_null_src (k INTEGER); + CREATE TABLE populated_src (k INTEGER); + + INSERT INTO customers VALUES (1,'US'),(2,'EU'),(3,'AP'); + -- customer_id has duplicates (1,1) and a NULL -> NOT unique. + INSERT INTO orders VALUES (1,1),(2,1),(3,2),(4,NULL); + -- customer_id unique -> one row per customer. + INSERT INTO user_profiles VALUES (1,'a'),(2,'b'),(3,'c'); + INSERT INTO carts VALUES (1),(2); + -- cart_id has duplicates -> NOT unique. + INSERT INTO cart_lines VALUES (1,1),(2,1),(3,2); + INSERT INTO left_tbl VALUES (1),(1),(2); + INSERT INTO right_tbl VALUES (1,'x'),(1,'y'),(3,'z'); + -- composite parent key is unique; child (a,b) has a dup + a NULL-key row. + INSERT INTO ck_parent VALUES (1,'x'),(1,'y'),(2,'x'); + INSERT INTO ck_child VALUES (1,'x'),(1,'x'),(2,'x'),(1,NULL); + -- empty_src / empty_tgt stay empty on purpose. + -- all_null_src has rows, but every key is NULL -> empty population. + INSERT INTO all_null_src VALUES (NULL),(NULL); + -- populated source pointing at an EMPTY target. + INSERT INTO populated_src VALUES (1),(2),(3); + """ + ) + conn.commit() + conn.close() + + +def _col(name: str, *, pk: bool = False, dtype: DataType = DataType.INT) -> Column: + return Column(name=name, sql=name, type=dtype, primary_key=pk) + + +def _models() -> list[SlayerModel]: + def m(name, table, cols, joins=None): + return SlayerModel( + name=name, sql_table=table, data_source="ds", columns=cols, joins=joins or [] + ) + + return [ + m("customers", "customers", [_col("id", pk=True), _col("region", dtype=DataType.TEXT)]), + m( + "orders", + "orders", + [_col("id", pk=True), _col("customer_id")], + [ModelJoin(target_model="customers", join_pairs=[["customer_id", "id"]])], + ), + m( + "user_profiles", + "user_profiles", + [_col("customer_id", pk=True), _col("bio", dtype=DataType.TEXT)], + [ModelJoin(target_model="customers", join_pairs=[["customer_id", "id"]])], + ), + m("cart_lines", "cart_lines", [_col("id", pk=True), _col("cart_id")]), + m( + "carts", + "carts", + [_col("id", pk=True)], + [ModelJoin(target_model="cart_lines", join_pairs=[["id", "cart_id"]])], + ), + m("right_m", "right_tbl", [_col("k"), _col("label", dtype=DataType.TEXT)]), + m( + "left_m", + "left_tbl", + [_col("k")], + [ModelJoin(target_model="right_m", join_pairs=[["k", "k"]])], + ), + m("empty_tgt", "empty_tgt", [_col("k", pk=True)]), + m( + "empty_src", + "empty_src", + [_col("k")], + [ModelJoin(target_model="empty_tgt", join_pairs=[["k", "k"]])], + ), + m( + "populated_src", + "populated_src", + [_col("k")], + [ModelJoin(target_model="empty_tgt", join_pairs=[["k", "k"]])], + ), + m( + "all_null_src", + "all_null_src", + [_col("k")], + [ModelJoin(target_model="customers", join_pairs=[["k", "id"]])], + ), + m("ck_parent", "ck_parent", [_col("a"), _col("b", dtype=DataType.TEXT)]), + m( + "ck_child", + "ck_child", + [_col("a"), _col("b", dtype=DataType.TEXT)], + [ + ModelJoin( + target_model="ck_parent", + join_pairs=[["a", "a"], ["b", "b"]], + ) + ], + ), + ] + + +async def _build_engine(workspace: Path) -> tuple[SlayerQueryEngine, YAMLStorage, DatasourceConfig]: + db = str(workspace / "d.db") + _seed_db(db) + storage = YAMLStorage(base_dir=str(workspace / "storage")) + ds = DatasourceConfig(name="ds", type="sqlite", database=db) + await storage.save_datasource(ds) + for model in _models(): + await storage.save_model(model) + return SlayerQueryEngine(storage=storage), storage, ds + + +def _find(report: JoinCardinalityReport, model: str, target: str): + return next( + f for f in report.findings if f.model == model and f.target_model == target + ) + + +# --------------------------------------------------------------------------- +# Classification from data +# --------------------------------------------------------------------------- + + +class TestClassification: + async def test_many_to_one(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "orders", "customers") + assert f.detected is JoinCardinality.MANY_TO_ONE + assert f.source_side.observed_unique is False + assert f.target_side.observed_unique is True + + async def test_one_to_one(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "user_profiles", "customers") + assert f.detected is JoinCardinality.ONE_TO_ONE + + async def test_one_to_many(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "carts", "cart_lines") + assert f.detected is JoinCardinality.ONE_TO_MANY + + async def test_many_to_many(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "left_m", "right_m") + assert f.detected is JoinCardinality.MANY_TO_MANY + + async def test_null_keys_excluded_from_population(self, workspace: Path) -> None: + # orders.customer_id has a NULL row; non-null population is 3 rows, + # 2 distinct -> not unique -> many_to_one still holds. + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "orders", "customers") + assert f.source_side.row_count == 3 + assert f.source_side.distinct_count == 2 + + async def test_composite_key_many_to_one(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "ck_child", "ck_parent") + assert f.detected is JoinCardinality.MANY_TO_ONE + + async def test_composite_key_null_row_excluded(self, workspace: Path) -> None: + # ck_child has 4 rows but one has a NULL in the (a,b) key -> the + # non-null population is 3 tuples, 2 distinct. + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "ck_child", "ck_parent") + assert f.source_side.row_count == 3 + assert f.source_side.distinct_count == 2 + + +# --------------------------------------------------------------------------- +# Verdicts +# --------------------------------------------------------------------------- + + +class TestVerdicts: + async def test_fills_none_when_stored_absent(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "orders", "customers") + assert f.stored is None + assert f.verdict is CardinalityVerdict.FILLS_NONE + + async def test_confirms_matching_stored(self, workspace: Path) -> None: + engine, storage, _ = await _build_engine(workspace) + orders = await storage.get_model("orders", data_source="ds") + orders.joins[0] = orders.joins[0].model_copy( + update={"cardinality": JoinCardinality.MANY_TO_ONE} + ) + await storage.save_model(orders) + + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "orders", "customers") + assert f.verdict is CardinalityVerdict.CONFIRMS + + async def test_contradicts_hard_when_data_disproves(self, workspace: Path) -> None: + engine, storage, _ = await _build_engine(workspace) + # Store a wrong one_to_one: it claims the source side is unique, but + # orders.customer_id has duplicates -> data disproves it. + orders = await storage.get_model("orders", data_source="ds") + orders.joins[0] = orders.joins[0].model_copy( + update={"cardinality": JoinCardinality.ONE_TO_ONE} + ) + await storage.save_model(orders) + + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "orders", "customers") + assert f.detected is JoinCardinality.MANY_TO_ONE + assert f.verdict is CardinalityVerdict.CONTRADICTS_HARD + + async def test_refines_when_change_is_not_a_hard_disproof( + self, workspace: Path + ) -> None: + engine, storage, _ = await _build_engine(workspace) + # Stored many_to_many claims the target side is non-unique. The data + # shows the target IS unique, but "no duplicates observed" does NOT + # disprove a non-uniqueness claim -> a soft REFINES, not a hard + # contradiction. + orders = await storage.get_model("orders", data_source="ds") + orders.joins[0] = orders.joins[0].model_copy( + update={"cardinality": JoinCardinality.MANY_TO_MANY} + ) + await storage.save_model(orders) + + report = await engine.detect_join_cardinality(data_source="ds", model="orders") + f = _find(report, "orders", "customers") + assert f.detected is JoinCardinality.MANY_TO_ONE + assert f.verdict is CardinalityVerdict.REFINES + + async def test_skipped_unsupported_for_expression_join_key( + self, workspace: Path + ) -> None: + engine, storage, _ = await _build_engine(workspace) + # A join key backed by a non-bare SQL expression is out of scope for + # v1 profiling (can't DISTINCT a physical column). + expr = SlayerModel( + name="orders_expr", + sql_table="orders", + data_source="ds", + columns=[ + _col("id", pk=True), + Column(name="ck", sql="customer_id + 0", type=DataType.INT), + ], + joins=[ModelJoin(target_model="customers", join_pairs=[["ck", "id"]])], + ) + await storage.save_model(expr) + + report = await engine.detect_join_cardinality( + data_source="ds", model="orders_expr" + ) + f = _find(report, "orders_expr", "customers") + assert f.verdict is CardinalityVerdict.SKIPPED_UNSUPPORTED + assert f.detected is None + assert f.note + + async def test_skipped_unsupported_for_sql_mode_model( + self, workspace: Path + ) -> None: + engine, storage, _ = await _build_engine(workspace) + # A sql-mode source model with a join is out of scope for v1 profiling. + raw = SlayerModel( + name="raw_orders", + sql="SELECT id, customer_id FROM orders", + data_source="ds", + columns=[_col("id", pk=True), _col("customer_id")], + joins=[ModelJoin(target_model="customers", join_pairs=[["customer_id", "id"]])], + ) + await storage.save_model(raw) + + report = await engine.detect_join_cardinality(data_source="ds", model="raw_orders") + f = _find(report, "raw_orders", "customers") + assert f.verdict is CardinalityVerdict.SKIPPED_UNSUPPORTED + assert f.detected is None + assert f.note # explains why it was skipped + + +# --------------------------------------------------------------------------- +# Declared-unique contradictions (reported, never mutated) +# --------------------------------------------------------------------------- + + +class TestUniqueContradictions: + async def test_reports_declared_unique_with_dups(self, workspace: Path) -> None: + engine, storage, _ = await _build_engine(workspace) + # Declare orders.customer_id unique (wrong — it has duplicates). + orders = await storage.get_model("orders", data_source="ds") + cc = next(c for c in orders.columns if c.name == "customer_id") + idx = orders.columns.index(cc) + orders.columns[idx] = cc.model_copy(update={"unique": True}) + await storage.save_model(orders) + + report = await engine.detect_join_cardinality(data_source="ds", model="orders") + f = _find(report, "orders", "customers") + assert any("customer_id" in c for c in f.unique_contradictions) + + # The declared flag is NOT mutated by detection. + reloaded = await storage.get_model("orders", data_source="ds") + assert next(c for c in reloaded.columns if c.name == "customer_id").unique is True + + async def test_sole_pk_column_with_dups_is_reported(self, workspace: Path) -> None: + """A column that IS the whole primary key does claim solo uniqueness.""" + engine, storage, _ = await _build_engine(workspace) + # cart_lines.cart_id has dups; declare it the sole PK of a probe model. + probe = SlayerModel( + name="solo_pk_lines", + sql_table="cart_lines", + data_source="ds", + columns=[_col("cart_id", pk=True), _col("id")], + joins=[ModelJoin(target_model="carts", join_pairs=[["cart_id", "id"]])], + ) + await storage.save_model(probe) + + report = await engine.detect_join_cardinality( + data_source="ds", model="solo_pk_lines" + ) + f = _find(report, "solo_pk_lines", "carts") + assert any("cart_id" in c for c in f.unique_contradictions) + + async def test_composite_pk_member_with_dups_is_not_a_contradiction( + self, workspace: Path + ) -> None: + """A member of a COMPOSITE primary key claims nothing on its own. + + Regression: jaffle_shop ``supplies``, PK ``(id, sku)``, joined on + ``sku`` alone. + """ + engine, storage, _ = await _build_engine(workspace) + # ck_child.a has duplicates; both a and b are stamped PK (composite). + probe = SlayerModel( + name="ck_child_solo", + sql_table="ck_child", + data_source="ds", + columns=[_col("a", pk=True), _col("b", pk=True, dtype=DataType.TEXT)], + joins=[ModelJoin(target_model="customers", join_pairs=[["a", "id"]])], + ) + await storage.save_model(probe) + + report = await engine.detect_join_cardinality( + data_source="ds", model="ck_child_solo" + ) + f = _find(report, "ck_child_solo", "customers") + # 'a' has dups, but it is only half of the composite key — no claim broken. + assert not [c for c in f.unique_contradictions if "ck_child_solo.a" in c] + + +# --------------------------------------------------------------------------- +# Persistence (report-only default; opt-in write) +# --------------------------------------------------------------------------- + + +class TestPersistence: + async def test_persist_false_does_not_write(self, workspace: Path) -> None: + engine, storage, _ = await _build_engine(workspace) + await engine.detect_join_cardinality(data_source="ds", persist=False) + orders = await storage.get_model("orders", data_source="ds") + assert orders.joins[0].cardinality is None + + async def test_persist_true_writes_detected_per_join( + self, workspace: Path + ) -> None: + engine, storage, _ = await _build_engine(workspace) + await engine.detect_join_cardinality(data_source="ds", persist=True) + + orders = await storage.get_model("orders", data_source="ds") + assert orders.joins[0].cardinality is JoinCardinality.MANY_TO_ONE + # Correct per-(model, join) identity — a different join gets its own value. + profiles = await storage.get_model("user_profiles", data_source="ds") + assert profiles.joins[0].cardinality is JoinCardinality.ONE_TO_ONE + + async def test_model_filter_scopes_scan(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds", model="orders") + assert {f.model for f in report.findings} == {"orders"} + + +# --------------------------------------------------------------------------- +# Row-level security: profiling must observe the tenant-scoped rows only +# --------------------------------------------------------------------------- + + +class TestSessionPolicyAppliedToProfiling: + """A configured SessionPolicy must scope the profiling scans too.""" + + async def test_profiling_sql_is_tenant_scoped(self, workspace: Path) -> None: + """Assert at the EXECUTION boundary, not at ``_apply_policy``'s return. + + Spying on the rewrite only proves it was computed, not submitted. + """ + _, storage, _ = await _build_engine(workspace) + # `region` exists on customers but not orders; "pass" lets the tables + # that lack it through so we can observe the rewrite on the one that + # has it. + policy = SessionPolicy( + ruleset=ColumnFilterRuleset( + column="region", value="US", on_unapplicable="pass" + ) + ) + scoped = SlayerQueryEngine(storage=storage, policy=policy) + + executed: list[str] = [] + real_client_cls = SlayerSQLClient + + class _RecordingClient(real_client_cls): + async def execute(self, sql=None, **kwargs): + executed.append(sql if sql is not None else kwargs.get("sql", "")) + return await super().execute(sql=sql, **kwargs) if sql is not None \ + else await super().execute(**kwargs) + + with patch( + "slayer.engine.query_engine.SlayerSQLClient", _RecordingClient + ): + report = await scoped.detect_join_cardinality( + data_source="ds", model="orders" + ) + + # Two scans per side, two sides. + assert len(executed) == 4 + customers_scans = [q for q in executed if "customers" in q] + assert len(customers_scans) == 2 + # BOTH customer-side scans (row count AND distinct) carry the filter. + for q in customers_scans: + assert "'US'" in q, f"unscoped SQL reached the datasource: {q}" + + # And the scoped statistics are what the report actually contains: + # only 1 of the 3 customers is in region 'US'. + f = _find(report, "orders", "customers") + assert f.target_side.row_count == 1 + assert f.target_side.distinct_count == 1 + + async def test_policy_failure_is_not_bypassed(self, workspace: Path) -> None: + """Fail-closed must propagate — detection must not sidestep the policy. + + Before the profiling scans were routed through ``_apply_policy`` this + silently succeeded, scanning every row regardless of the policy. + """ + _, storage, _ = await _build_engine(workspace) + policy = SessionPolicy( + ruleset=ColumnFilterRuleset(column="tenant_id", value="t1") + ) + scoped = SlayerQueryEngine(storage=storage, policy=policy) + with pytest.raises(ForcedFilterError): + await scoped.detect_join_cardinality(data_source="ds", model="orders") + + async def test_no_policy_leaves_sql_untouched(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality( + data_source="ds", model="orders" + ) + # Unscoped engine still profiles normally (zero-overhead no-op path). + f = _find(report, "orders", "customers") + assert f.detected is JoinCardinality.MANY_TO_ONE + + +class TestSideStatsSqlShape: + """The profiling SQL is built via sqlglot, not string concatenation.""" + + def test_identifiers_are_quoted_and_nulls_excluded(self) -> None: + rows_sql, dist_sql = SlayerQueryEngine._side_stats_sql( + table="public.orders", key_cols=["customer_id"], sqlglot_name="postgres", + ) + for sql in (rows_sql, dist_sql): + assert 'NOT "customer_id" IS NULL' in sql + assert '"public"."orders"' in sql + assert "DISTINCT" in dist_sql + assert "DISTINCT" not in rows_sql + + def test_composite_keys_exclude_nulls_on_every_column(self) -> None: + rows_sql, dist_sql = SlayerQueryEngine._side_stats_sql( + table="t", key_cols=["a", "b"], sqlglot_name="postgres", + ) + for sql in (rows_sql, dist_sql): + assert 'NOT "a" IS NULL' in sql + assert 'NOT "b" IS NULL' in sql + + def test_hostile_identifier_stays_a_single_identifier(self) -> None: + payload = 'a"; DROP TABLE users; --' + rows_sql, _ = SlayerQueryEngine._side_stats_sql( + table="t", key_cols=[payload, "b"], sqlglot_name="postgres", + ) + # sqlglot doubles the embedded quote, so the payload cannot terminate + # the identifier and start a new statement. + assert 'a""; DROP TABLE users; --' in rows_sql + # The only statement is the SELECT: nothing escaped the quoting. + assert rows_sql.strip().startswith("SELECT") + assert 'NOT "b" IS NULL' in rows_sql + + +class TestEmptyPopulationIsNoEvidence: + """An empty key population proves nothing about arity. + + 0 == 0 reads as observed_unique, so without a guard two empty tables would + "detect" one_to_one and persist it. + """ + + async def test_empty_tables_detect_nothing(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "empty_src", "empty_tgt") + assert f.detected is None + assert f.verdict is CardinalityVerdict.NO_EVIDENCE + assert "no evidence" in (f.note or "") + # The observed stats are still reported for transparency. + assert f.source_side.row_count == 0 + assert f.target_side.row_count == 0 + + async def test_all_null_keys_are_an_empty_population( + self, workspace: Path + ) -> None: + # The table HAS rows, but every key is NULL, so the profiled + # population is empty just the same. + engine, _, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality(data_source="ds") + f = _find(report, "all_null_src", "customers") + assert f.detected is None + assert f.verdict is CardinalityVerdict.NO_EVIDENCE + assert f.source_side.row_count == 0 + # The non-empty target side is still profiled and reported. + assert f.target_side.row_count == 3 + + async def test_populated_source_with_empty_target_detects_nothing( + self, workspace: Path + ) -> None: + """The TARGET side alone being empty is equally no evidence. + + Guards the asymmetric case: an implementation that checked only + `source_side.row_count` would infer (and persist) an arity here off a + target scan that read nothing. + """ + engine, storage, _ = await _build_engine(workspace) + report = await engine.detect_join_cardinality( + data_source="ds", persist=True + ) + f = _find(report, "populated_src", "empty_tgt") + assert f.verdict is CardinalityVerdict.NO_EVIDENCE + assert f.detected is None + # The source really was populated — only the target was empty. + assert f.source_side.row_count == 3 + assert f.target_side.row_count == 0 + # persist=True must not have written an arity. + reloaded = await storage.get_model("populated_src", data_source="ds") + j = next(j for j in reloaded.joins if j.target_model == "empty_tgt") + assert j.cardinality is None + + async def test_empty_side_is_never_persisted(self, workspace: Path) -> None: + engine, storage, _ = await _build_engine(workspace) + await engine.detect_join_cardinality(data_source="ds", persist=True) + reloaded = await storage.get_model("empty_src", data_source="ds") + j = next(j for j in reloaded.joins if j.target_model == "empty_tgt") + assert j.cardinality is None, "an empty scan must not write an arity" + + + async def test_no_evidence_is_distinct_from_skipped_unsupported( + self, workspace: Path + ) -> None: + """The two non-detecting verdicts must stay tellable apart. + + `no_evidence` is retryable once data lands; `skipped_unsupported` is a + shape that can never be profiled. + """ + engine, storage, _ = await _build_engine(workspace) + raw = SlayerModel( + name="raw_empty", + sql="SELECT k FROM empty_src", + data_source="ds", + columns=[_col("k")], + joins=[ModelJoin(target_model="empty_tgt", join_pairs=[["k", "k"]])], + ) + await storage.save_model(raw) + report = await engine.detect_join_cardinality(data_source="ds") + + empty = _find(report, "empty_src", "empty_tgt") + unsupported = _find(report, "raw_empty", "empty_tgt") + assert empty.verdict is CardinalityVerdict.NO_EVIDENCE + assert unsupported.verdict is CardinalityVerdict.SKIPPED_UNSUPPORTED + assert empty.verdict != unsupported.verdict + # Neither detects a value. + assert empty.detected is None + assert unsupported.detected is None + + +class TestScanFailureIsContained: + """One unreadable join must not abort the whole report.""" + + async def test_failed_scan_becomes_a_finding(self, workspace: Path) -> None: + engine, _, _ = await _build_engine(workspace) + real_side_stats = SlayerQueryEngine._side_stats + + async def _flaky(self, *, client, table, key_cols, sqlglot_name, datasource): + if table == "cart_lines": + raise RuntimeError("table is on fire") + return await real_side_stats( + self, client=client, table=table, key_cols=key_cols, + sqlglot_name=sqlglot_name, datasource=datasource, + ) + + with patch.object(SlayerQueryEngine, "_side_stats", _flaky): + report = await engine.detect_join_cardinality(data_source="ds") + + failed = _find(report, "carts", "cart_lines") + assert failed.verdict is CardinalityVerdict.SCAN_FAILED + assert failed.detected is None + assert "table is on fire" in (failed.note or "") + + # Every other join still reports. + healthy = _find(report, "orders", "customers") + assert healthy.detected is JoinCardinality.MANY_TO_ONE + + async def test_failed_scan_persists_nothing(self, workspace: Path) -> None: + engine, storage, _ = await _build_engine(workspace) + + async def _boom(self, **kwargs): + raise RuntimeError("nope") + + with patch.object(SlayerQueryEngine, "_side_stats", _boom): + await engine.detect_join_cardinality(data_source="ds", persist=True) + + reloaded = await storage.get_model("carts", data_source="ds") + assert all(j.cardinality is None for j in reloaded.joins) diff --git a/tests/test_ingestion.py b/tests/test_ingestion.py index 591807a5..7671ac54 100644 --- a/tests/test_ingestion.py +++ b/tests/test_ingestion.py @@ -208,12 +208,12 @@ class TestGenerateJoinsDedup: def test_multiple_fks_to_same_target_preserved(self): """Two distinct FKs to the same target table should both produce joins.""" inspector = MagicMock(spec=sa.engine.Inspector) - fk_rels = [ - ("buyer_id", "users", "id"), - ("seller_id", "users", "id"), + fk_groups = [ + ("users", [("buyer_id", "id")]), + ("users", [("seller_id", "id")]), ] with patch( - "slayer.engine.ingestion._get_fk_relationships", return_value=fk_rels, + "slayer.engine.ingestion._get_fk_constraint_groups", return_value=fk_groups, ): joins = _generate_joins( inspector=inspector, @@ -230,12 +230,12 @@ def test_multiple_fks_to_same_target_preserved(self): def test_exact_duplicate_fk_deduplicated(self): """Identical FK pair to the same target should be deduplicated.""" inspector = MagicMock(spec=sa.engine.Inspector) - fk_rels = [ - ("buyer_id", "users", "id"), - ("buyer_id", "users", "id"), + fk_groups = [ + ("users", [("buyer_id", "id")]), + ("users", [("buyer_id", "id")]), ] with patch( - "slayer.engine.ingestion._get_fk_relationships", return_value=fk_rels, + "slayer.engine.ingestion._get_fk_constraint_groups", return_value=fk_groups, ): joins = _generate_joins( inspector=inspector, diff --git a/tests/test_ingestion_cardinality.py b/tests/test_ingestion_cardinality.py new file mode 100644 index 00000000..d2c245d3 --- /dev/null +++ b/tests/test_ingestion_cardinality.py @@ -0,0 +1,466 @@ +"""Ingestion: composite-FK grouping, structural cardinality, and ``Column.unique``.""" + +from __future__ import annotations + +import sqlite3 +import tempfile +from pathlib import Path + +import pytest +import sqlalchemy as sa + +from slayer.core.enums import JoinCardinality +from slayer.core.models import DatasourceConfig +from slayer.engine.ingestion import ( + _build_fk_graph, + _generate_joins, + _get_single_column_unique_names, + _is_cross_schema_fk, + _is_partial_index, + _pk_key_sets, + _safe_get_pk_constraint, + _unique_index_key_sets, + ingest_datasource_idempotent, +) +from slayer.storage.yaml_storage import YAMLStorage + + +@pytest.fixture +def workspace(): + tmp = tempfile.TemporaryDirectory() + try: + yield Path(tmp.name) + finally: + tmp.cleanup() + + +def _create_schema(db_path: str) -> None: + conn = sqlite3.connect(db_path) + conn.executescript( + """ + CREATE TABLE customers ( + id INTEGER PRIMARY KEY, + email TEXT UNIQUE, + region TEXT NOT NULL + ); + CREATE TABLE orders ( + id INTEGER PRIMARY KEY, + amount REAL NOT NULL, + customer_id INTEGER REFERENCES customers(id) + ); + -- one-to-one: the FK source column is itself the PK. + CREATE TABLE user_profiles ( + customer_id INTEGER PRIMARY KEY REFERENCES customers(id), + bio TEXT + ); + -- composite FK target. + CREATE TABLE org_units ( + org_id INTEGER, + code TEXT, + name TEXT NOT NULL, + PRIMARY KEY (org_id, code) + ); + CREATE TABLE memberships ( + id INTEGER PRIMARY KEY, + org_id INTEGER, + code TEXT, + FOREIGN KEY (org_id, code) REFERENCES org_units(org_id, code) + ); + -- one-to-one via a non-PK UNIQUE source column. + CREATE TABLE accounts ( + id INTEGER PRIMARY KEY, + customer_id INTEGER UNIQUE REFERENCES customers(id), + balance REAL + ); + INSERT INTO customers VALUES (1, 'a@x.com', 'US'), (2, 'b@x.com', 'EU'); + INSERT INTO orders VALUES (1, 100.0, 1), (2, 50.0, 1); + INSERT INTO user_profiles VALUES (1, 'hi'), (2, 'yo'); + INSERT INTO org_units VALUES (1, 'A', 'Alpha'), (1, 'B', 'Beta'); + INSERT INTO memberships VALUES (1, 1, 'A'), (2, 1, 'A'); + INSERT INTO accounts VALUES (1, 1, 10.0), (2, 2, 20.0); + """ + ) + conn.commit() + conn.close() + + +async def _setup(workspace: Path) -> tuple: + db_path = str(workspace / "live.db") + _create_schema(db_path) + storage = YAMLStorage(base_dir=str(workspace / "storage")) + ds = DatasourceConfig(name="ds", type="sqlite", database=db_path) + await storage.save_datasource(ds) + await ingest_datasource_idempotent(datasource=ds, storage=storage) + return storage, ds, db_path + + +def _join_to(model, target: str): + return [j for j in model.joins if j.target_model == target] + + +# --------------------------------------------------------------------------- +# Composite-FK fix +# --------------------------------------------------------------------------- + + +class TestCompositeFk: + async def test_composite_fk_becomes_single_join_with_all_pairs( + self, workspace: Path + ) -> None: + storage, _, _ = await _setup(workspace) + mem = await storage.get_model("memberships", data_source="ds") + assert mem is not None + org_joins = _join_to(mem, "org_units") + # Exactly ONE join, carrying BOTH key pairs — not two single-col joins. + assert len(org_joins) == 1 + assert {tuple(p) for p in org_joins[0].join_pairs} == { + ("org_id", "org_id"), + ("code", "code"), + } + + async def test_generate_joins_groups_composite_fk(self, workspace: Path) -> None: + db_path = str(workspace / "live.db") + _create_schema(db_path) + eng = sa.create_engine(f"sqlite:///{db_path}") + insp = sa.inspect(eng) + table_set = { + "customers", + "orders", + "user_profiles", + "org_units", + "memberships", + } + joins = _generate_joins( + inspector=insp, + source_table="memberships", + referenced_tables={"org_units"}, + schema=None, + table_set=table_set, + ) + org_joins = [j for j in joins if j.target_model == "org_units"] + assert len(org_joins) == 1 + assert {tuple(p) for p in org_joins[0].join_pairs} == { + ("org_id", "org_id"), + ("code", "code"), + } + + async def test_build_fk_graph_one_edge_per_group(self, workspace: Path) -> None: + db_path = str(workspace / "live.db") + _create_schema(db_path) + eng = sa.create_engine(f"sqlite:///{db_path}") + insp = sa.inspect(eng) + graph = _build_fk_graph( + inspector=insp, + table_names=[ + "customers", + "orders", + "user_profiles", + "org_units", + "memberships", + ], + schema=None, + ) + # Composite FK contributes a single edge memberships -> org_units. + assert graph.get("memberships") == {"org_units"} + assert graph.get("orders") == {"customers"} + assert graph.get("user_profiles") == {"customers"} + + +# --------------------------------------------------------------------------- +# Structural cardinality inference +# --------------------------------------------------------------------------- + + +class TestStructuralCardinality: + async def test_fk_join_defaults_many_to_one(self, workspace: Path) -> None: + storage, _, _ = await _setup(workspace) + orders = await storage.get_model("orders", data_source="ds") + j = _join_to(orders, "customers")[0] + assert j.cardinality is JoinCardinality.MANY_TO_ONE + + async def test_pk_source_fk_join_is_one_to_one(self, workspace: Path) -> None: + storage, _, _ = await _setup(workspace) + profiles = await storage.get_model("user_profiles", data_source="ds") + j = _join_to(profiles, "customers")[0] + # user_profiles.customer_id is the PK (source unique) and customers.id is + # the PK (target unique) => one_to_one. + assert j.cardinality is JoinCardinality.ONE_TO_ONE + + async def test_composite_fk_join_many_to_one(self, workspace: Path) -> None: + storage, _, _ = await _setup(workspace) + mem = await storage.get_model("memberships", data_source="ds") + j = _join_to(mem, "org_units")[0] + assert j.cardinality is JoinCardinality.MANY_TO_ONE + + async def test_non_pk_unique_source_fk_is_one_to_one( + self, workspace: Path + ) -> None: + # accounts.customer_id is a non-PK UNIQUE column referencing the + # customers PK => both sides unique => one_to_one. + storage, _, _ = await _setup(workspace) + accounts = await storage.get_model("accounts", data_source="ds") + j = _join_to(accounts, "customers")[0] + assert j.cardinality is JoinCardinality.ONE_TO_ONE + + +# --------------------------------------------------------------------------- +# Column.unique population +# --------------------------------------------------------------------------- + + +class TestColumnUnique: + async def test_unique_constraint_sets_unique_flag(self, workspace: Path) -> None: + storage, _, _ = await _setup(workspace) + customers = await storage.get_model("customers", data_source="ds") + email = next(c for c in customers.columns if c.name == "email") + assert email.unique is True + + async def test_non_unique_column_stays_false(self, workspace: Path) -> None: + storage, _, _ = await _setup(workspace) + customers = await storage.get_model("customers", data_source="ds") + region = next(c for c in customers.columns if c.name == "region") + assert region.unique is False + + async def test_pk_column_marked_primary_key_not_redundant_unique( + self, workspace: Path + ) -> None: + storage, _, _ = await _setup(workspace) + customers = await storage.get_model("customers", data_source="ds") + id_col = next(c for c in customers.columns if c.name == "id") + assert id_col.primary_key is True + # primary_key is the canonical marker — unique is NOT redundantly stamped. + assert id_col.unique is False + + def test_expression_index_is_not_a_single_column_claim(self) -> None: + """A unique EXPRESSION index must not collapse to a solo-unique claim. + + Members reflect as ``None``, so compacting them would turn unique + ``(email, lower(name))`` into a bogus claim on ``email``. + """ + + class _FakeInspector: + def get_unique_constraints(self, table_name, schema=None): + return [] + + def get_indexes(self, table_name, schema=None): + return [ + # (email, ) — unique on the PAIR, not on email. + {"unique": True, "column_names": ["email", None]}, + # A genuine single-column unique index. + {"unique": True, "column_names": ["slug"]}, + # Non-unique index is ignored entirely. + {"unique": False, "column_names": ["region"]}, + ] + + insp = _FakeInspector() + assert _unique_index_key_sets(insp, "t", None) == [["slug"]] + assert _get_single_column_unique_names( + insp, "t", None, pk_cols=set() + ) == {"slug"} + + def test_partial_unique_index_is_not_a_uniqueness_claim(self) -> None: + """A predicate-filtered unique index constrains only matching rows. + + The soft-delete pattern: `UNIQUE INDEX ... WHERE deleted_at IS NULL`. + """ + + class _FakeInspector: + def get_unique_constraints(self, table_name, schema=None): + return [] + + def get_indexes(self, table_name, schema=None): + return [ + { + "unique": True, + "column_names": ["email"], + "dialect_options": {"postgresql_where": "deleted_at IS NULL"}, + }, + {"unique": True, "column_names": ["slug"]}, + # An empty predicate is not a predicate. + { + "unique": True, + "column_names": ["ref"], + "dialect_options": {"postgresql_where": None}, + }, + ] + + insp = _FakeInspector() + assert _unique_index_key_sets(insp, "t", None) == [["slug"], ["ref"]] + assert _get_single_column_unique_names( + insp, "t", None, pk_cols=set() + ) == {"slug", "ref"} + + +class TestCrossSchemaFk: + """A cross-schema FK has no model to bind to and must not be guessed.""" + + def test_cross_schema_fk_is_skipped(self) -> None: + fk = { + "referred_table": "customers", + "referred_schema": "other", + "constrained_columns": ["customer_id"], + "referred_columns": ["id"], + } + assert _is_cross_schema_fk(fk, "public") is True + + def test_same_schema_fk_is_kept(self) -> None: + fk = {"referred_table": "customers", "referred_schema": "public"} + assert _is_cross_schema_fk(fk, "public") is False + + def test_null_referred_schema_is_kept(self) -> None: + # Same-schema FKs commonly report referred_schema=None. + fk = {"referred_table": "customers", "referred_schema": None} + assert _is_cross_schema_fk(fk, "public") is False + + def test_schemaless_backend_is_kept(self) -> None: + # SQLite and friends have no schema at all. + fk = {"referred_table": "customers", "referred_schema": None} + assert _is_cross_schema_fk(fk, None) is False + + def test_default_schema_ingest_still_skips_cross_schema(self) -> None: + """Ingesting the default schema passes schema=None, so the fallback to + default_schema_name is what stops a cross-schema FK slipping through. + """ + fk = {"referred_table": "customers", "referred_schema": "archive"} + assert _is_cross_schema_fk(fk, None, "public") is True + + def test_default_schema_ingest_keeps_same_schema_fk(self) -> None: + fk = {"referred_table": "customers", "referred_schema": "public"} + assert _is_cross_schema_fk(fk, None, "public") is False + + def test_explicit_target_schema_is_skipped_when_ingest_schema_unknown( + self, + ) -> None: + """Fail safe: an explicit target schema we cannot confirm is skipped.""" + fk = {"referred_table": "customers", "referred_schema": "archive"} + assert _is_cross_schema_fk(fk, None, None) is True + # Also with neither the ingested schema nor a default available. + assert _is_cross_schema_fk(fk, None) is True + + def test_absent_referred_schema_is_always_kept(self) -> None: + # No explicit target schema -> nothing to disagree with, at any + # combination of ingested/default schema. + fk = {"referred_table": "customers", "referred_schema": None} + assert _is_cross_schema_fk(fk, None, None) is False + assert _is_cross_schema_fk(fk, "public", None) is False + assert _is_cross_schema_fk(fk, None, "public") is False + + def test_explicit_schema_wins_over_default(self) -> None: + fk = {"referred_table": "customers", "referred_schema": "archive"} + assert _is_cross_schema_fk(fk, "archive", "public") is False + + def test_cross_schema_fk_excluded_from_generated_joins(self) -> None: + """End-to-end: the wrong same-named table is not joined to.""" + + class _FakeInspector: + def get_foreign_keys(self, table_name, schema=None): + return [ + { + "referred_table": "customers", + "referred_schema": "archive", # NOT the ingested schema + "constrained_columns": ["customer_id"], + "referred_columns": ["id"], + }, + ] + + def get_pk_constraint(self, table_name, schema=None): + return {"constrained_columns": ["id"]} + + def get_unique_constraints(self, table_name, schema=None): + return [] + + def get_indexes(self, table_name, schema=None): + return [] + + joins = _generate_joins( + _FakeInspector(), "orders", {"customers"}, "public", {"orders", "customers"}, + ) + assert joins == [] + + +class TestSafePkConstraintContract: + """`_safe_get_pk_constraint` is annotated `-> dict` and four of its five + callers do an unguarded `.get()`, so every path must honour that.""" + + class _Eng: + class dialect: + name = "sqlite" + + def _insp(self, result): + class _I: + def get_pk_constraint(self, table_name, schema=None): + if isinstance(result, Exception): + raise result + return result + + return _I() + + def test_sqlite_none_result_normalized(self) -> None: + pk = _safe_get_pk_constraint( + self._insp(None), self._Eng(), "t", None + ) + assert pk == {"constrained_columns": []} + assert pk.get("constrained_columns") == [] # caller pattern must work + + def test_sqlite_non_mapping_result_normalized(self) -> None: + pk = _safe_get_pk_constraint( + self._insp(["not", "a", "mapping"]), self._Eng(), "t", None + ) + assert pk == {"constrained_columns": []} + + def test_sqlite_raising_inspector_normalized(self) -> None: + pk = _safe_get_pk_constraint( + self._insp(RuntimeError("boom")), self._Eng(), "t", None + ) + assert pk == {"constrained_columns": []} + + def test_sqlite_valid_mapping_passes_through(self) -> None: + pk = _safe_get_pk_constraint( + self._insp({"constrained_columns": ["id"]}), self._Eng(), "t", None + ) + assert pk == {"constrained_columns": ["id"]} + + def test_pk_key_sets_handles_bare_inspector_non_mapping(self) -> None: + # sa_engine=None path goes straight to the inspector, un-normalized. + assert _pk_key_sets(self._insp(None), "t", None, None) == [] + assert _pk_key_sets(self._insp({"constrained_columns": ["id"]}), "t", None, None) == [["id"]] + + +class TestPartialIndexPredicateIsNeverEvaluated: + """`ColumnElement.__bool__` raises, and this runs outside _safe_introspect.""" + + class _Raising: + def __bool__(self): + raise TypeError("Boolean value of this clause is not defined") + + def test_expression_predicate_counts_as_partial_without_bool(self) -> None: + idx = { + "unique": True, + "column_names": ["email"], + "dialect_options": {"postgresql_where": self._Raising()}, + } + # Must not raise, and must classify as partial. + assert _is_partial_index(idx) is True + + def test_index_with_raising_predicate_is_skipped_not_fatal(self) -> None: + class _I: + def get_indexes(self, table_name, schema=None): + return [ + { + "unique": True, + "column_names": ["email"], + "dialect_options": { + "postgresql_where": TestPartialIndexPredicateIsNeverEvaluated._Raising() + }, + }, + {"unique": True, "column_names": ["slug"]}, + ] + + assert _unique_index_key_sets(_I(), "t", None) == [["slug"]] + + def test_empty_string_predicate_is_not_partial(self) -> None: + assert _is_partial_index( + {"dialect_options": {"postgresql_where": " "}} + ) is False + assert _is_partial_index({"dialect_options": {}}) is False + assert _is_partial_index({}) is False diff --git a/tests/test_ingestion_cardinality_reingest.py b/tests/test_ingestion_cardinality_reingest.py new file mode 100644 index 00000000..6ed3c9cf --- /dev/null +++ b/tests/test_ingestion_cardinality_reingest.py @@ -0,0 +1,237 @@ +"""Idempotent re-ingest must persist metadata-only cardinality/unique fills.""" + +from __future__ import annotations + +import sqlite3 +import tempfile +from pathlib import Path + +import pytest + +from slayer.core.enums import JoinCardinality +from slayer.core.models import DatasourceConfig +from slayer.engine.ingestion import ingest_datasource_idempotent +from slayer.storage.yaml_storage import YAMLStorage + + +@pytest.fixture +def workspace(): + tmp = tempfile.TemporaryDirectory() + try: + yield Path(tmp.name) + finally: + tmp.cleanup() + + +def _create_schema(db_path: str) -> None: + conn = sqlite3.connect(db_path) + conn.executescript( + """ + CREATE TABLE customers ( + id INTEGER PRIMARY KEY, + email TEXT UNIQUE, + region TEXT NOT NULL + ); + CREATE TABLE orders ( + id INTEGER PRIMARY KEY, + amount REAL NOT NULL, + customer_id INTEGER REFERENCES customers(id) + ); + INSERT INTO customers VALUES (1, 'a@x.com', 'US'); + INSERT INTO orders VALUES (1, 100.0, 1); + """ + ) + conn.commit() + conn.close() + + +async def _setup(workspace: Path) -> tuple: + db_path = str(workspace / "live.db") + _create_schema(db_path) + storage = YAMLStorage(base_dir=str(workspace / "storage")) + ds = DatasourceConfig(name="ds", type="sqlite", database=db_path) + await storage.save_datasource(ds) + await ingest_datasource_idempotent(datasource=ds, storage=storage) + return storage, ds + + +def _order_join(model): + return next(j for j in model.joins if j.target_model == "customers") + + +class TestFillsMetadataAndPersists: + async def test_reingest_refills_none_cardinality_and_persists( + self, workspace: Path + ) -> None: + storage, ds = await _setup(workspace) + + # Simulate legacy persisted data: join without a cardinality. + orders = await storage.get_model("orders", data_source="ds") + orders.joins[0] = _order_join(orders).model_copy(update={"cardinality": None}) + await storage.save_model(orders) + + await ingest_datasource_idempotent(datasource=ds, storage=storage) + + reloaded = await storage.get_model("orders", data_source="ds") + # Refilled AND persisted (reloaded from storage proves the save). + assert _order_join(reloaded).cardinality is JoinCardinality.MANY_TO_ONE + + async def test_reingest_sets_unique_and_persists(self, workspace: Path) -> None: + storage, ds = await _setup(workspace) + + customers = await storage.get_model("customers", data_source="ds") + email = next(c for c in customers.columns if c.name == "email") + idx = customers.columns.index(email) + customers.columns[idx] = email.model_copy(update={"unique": False}) + await storage.save_model(customers) + + await ingest_datasource_idempotent(datasource=ds, storage=storage) + + reloaded = await storage.get_model("customers", data_source="ds") + email = next(c for c in reloaded.columns if c.name == "email") + assert email.unique is True + + +class TestAdditiveContract: + async def test_reingest_does_not_overwrite_user_cardinality( + self, workspace: Path + ) -> None: + storage, ds = await _setup(workspace) + + # A deliberate user override that disagrees with the structural guess. + orders = await storage.get_model("orders", data_source="ds") + orders.joins[0] = _order_join(orders).model_copy( + update={"cardinality": JoinCardinality.ONE_TO_ONE} + ) + await storage.save_model(orders) + + await ingest_datasource_idempotent(datasource=ds, storage=storage) + + reloaded = await storage.get_model("orders", data_source="ds") + assert _order_join(reloaded).cardinality is JoinCardinality.ONE_TO_ONE + + async def test_reingest_does_not_downgrade_user_unique( + self, workspace: Path + ) -> None: + storage, ds = await _setup(workspace) + + # User marked a column unique that has no DB constraint. + customers = await storage.get_model("customers", data_source="ds") + region = next(c for c in customers.columns if c.name == "region") + idx = customers.columns.index(region) + customers.columns[idx] = region.model_copy(update={"unique": True}) + await storage.save_model(customers) + + await ingest_datasource_idempotent(datasource=ds, storage=storage) + + reloaded = await storage.get_model("customers", data_source="ds") + region = next(c for c in reloaded.columns if c.name == "region") + assert region.unique is True + + +class TestLegacyJoinTargetNormalisation: + """A join persisted with the live object name self-heals on re-ingest. + + Model names cannot contain ``__``, so such a target can never resolve — + it is repaired rather than kept alongside the corrected join. + """ + + async def test_legacy_double_underscore_target_is_rewritten( + self, workspace: Path + ) -> None: + import sqlite3 + + from slayer.core.models import DatasourceConfig + from slayer.storage.yaml_storage import YAMLStorage + + db = str(workspace / "legacy.db") + conn = sqlite3.connect(db) + conn.executescript( + """ + CREATE TABLE reports__patient__drug (id INTEGER PRIMARY KEY); + CREATE TABLE visits ( + id INTEGER PRIMARY KEY, + report_id INTEGER REFERENCES reports__patient__drug(id) + ); + """ + ) + conn.commit() + conn.close() + + storage = YAMLStorage(base_dir=str(workspace / "store")) + ds = DatasourceConfig(name="ds", type="sqlite", database=db) + await storage.save_datasource(ds) + await ingest_datasource_idempotent(datasource=ds, storage=storage) + + # Rewind to the pre-fix state: the join names the live object. + visits = await storage.get_model("visits", data_source="ds") + visits.joins[0].target_model = "reports__patient__drug" + await storage.save_model(visits) + + await ingest_datasource_idempotent(datasource=ds, storage=storage) + + reloaded = await storage.get_model("visits", data_source="ds") + assert [j.target_model for j in reloaded.joins] == ["reports_patient_drug"] + + async def test_colliding_legacy_targets_do_not_break_reingest( + self, workspace: Path + ) -> None: + """Two legacy targets can sanitize to the same name. + + Repairing on the name alone would point both at `a_b`, and the + duplicate-target guard in `_merge_joins_strict` would then turn a + tolerated (if dangling) store into a hard re-ingest failure. Only the + join whose pairs match the fresh one is the join the bug produced. + + Looped, so this also covers idempotence: a repaired target sanitizes + to itself and later runs are no-ops. + """ + import sqlite3 + + from slayer.core.enums import DataType + from slayer.core.models import ( + Column, + DatasourceConfig, + ModelJoin, + SlayerModel, + ) + from slayer.storage.yaml_storage import YAMLStorage + + db = str(workspace / "collide.db") + conn = sqlite3.connect(db) + conn.executescript( + """ + CREATE TABLE a_b (id INTEGER PRIMARY KEY); + CREATE TABLE src ( + id INTEGER PRIMARY KEY, + x INTEGER REFERENCES a_b(id) + ); + """ + ) + conn.commit() + conn.close() + + storage = YAMLStorage(base_dir=str(workspace / "store")) + ds = DatasourceConfig(name="ds", type="sqlite", database=db) + await storage.save_datasource(ds) + await storage.save_model(SlayerModel( + name="src", sql_table="src", data_source="ds", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="x", type=DataType.INT), + ], + joins=[ + # Matches the fresh join's pairs — this is the one to repair. + ModelJoin(target_model="a__b", join_pairs=[["x", "id"]]), + # Sanitizes to the same name but has different pairs, so it is + # somebody else's join and must be left alone. + ModelJoin(target_model="a___b", join_pairs=[["id", "id"]]), + ], + )) + + for _ in range(3): + await ingest_datasource_idempotent(datasource=ds, storage=storage) + reloaded = await storage.get_model("src", data_source="ds") + targets = [j.target_model for j in reloaded.joins] + assert targets == ["a_b", "a___b"], targets + assert len(targets) == len(set(targets)) diff --git a/tests/test_ingestion_name_sanitize.py b/tests/test_ingestion_name_sanitize.py index 76520a12..a15bf49d 100644 --- a/tests/test_ingestion_name_sanitize.py +++ b/tests/test_ingestion_name_sanitize.py @@ -457,3 +457,138 @@ def test_empty_schema_reports_no_objects(self, workspace: Path) -> None: assert report.models == [] assert report.skipped == [] assert report.objects == [] + + +# --------------------------------------------------------------------------- +# FK targets follow the sanitized model name +# --------------------------------------------------------------------------- + + +class TestJoinTargetsUseModelNames: + """A join must name the persisted MODEL, not the live object. + + ``__`` is sanitized out of model names, so an FK pointing at + ``reports__patient__drug`` has to bind to ``reports_patient_drug``. + """ + + def test_fk_to_sanitized_table_targets_the_model_name( + self, workspace: Path + ) -> None: + ds = _sqlite_ds( + workspace, + """ + CREATE TABLE reports__patient__drug (id INTEGER PRIMARY KEY); + CREATE TABLE visits ( + id INTEGER PRIMARY KEY, + report_id INTEGER REFERENCES reports__patient__drug(id) + ); + """, + ) + models = {m.name: m for m in ingest_datasource(datasource=ds)} + assert "reports_patient_drug" in models + + visits = models["visits"] + assert [j.target_model for j in visits.joins] == ["reports_patient_drug"] + # The whole point: the target resolves to a model that exists. + assert visits.joins[0].target_model in models + + def test_join_is_dropped_when_the_target_has_no_model( + self, workspace: Path + ) -> None: + """A collided target is skipped, so a join to it would dangle.""" + ds = _sqlite_ds( + workspace, + """ + CREATE TABLE a_b (id INTEGER PRIMARY KEY); + CREATE TABLE a__b (id INTEGER PRIMARY KEY); + CREATE TABLE refs_it ( + id INTEGER PRIMARY KEY, + x INTEGER REFERENCES a__b(id) + ); + """, + ) + report = ingest_datasource_report(datasource=ds) + models = {m.name: m for m in report.models} + assert "a__b" in _skipped_names(report) + + refs_it = models["refs_it"] + assert all(j.target_model in models for j in refs_it.joins) + + +class TestEmptyJoinListIsNotNoJoinList: + """An empty join list means every join was dropped, not "none generated". + + Conflating the two sent the fallback to introspect the skipped object and + emit `a__b.label` — a column name the SQL generator reads as a join path. + (`_columns_to_model` drops dotted names, so no bad model reached storage; + the cost was pointless introspection against an object with no model.) + """ + + def _fixture(self, workspace: Path): + ds = _sqlite_ds( + workspace, + """ + CREATE TABLE a_b (id INTEGER PRIMARY KEY); + CREATE TABLE a__b (id INTEGER PRIMARY KEY, label TEXT); + CREATE TABLE refs_it ( + id INTEGER PRIMARY KEY, + x INTEGER REFERENCES a__b(id) + ); + """, + ) + sa_engine = engine_factory.get_engine(ds.resolve_env_vars()) + return sa_engine, sa.inspect(sa_engine) + + def _introspect(self, workspace: Path, joins): + from slayer.engine.ingestion import _introspect_query_columns_via_inspector + + sa_engine, inspector = self._fixture(workspace) + return [ + c[0] + for c in _introspect_query_columns_via_inspector( + sa_engine=sa_engine, + inspector=inspector, + table_name="refs_it", + schema=None, + rollup_sql=None, + referenced_tables={"a__b"}, + fk_columns_by_table={"refs_it": {"x"}}, + joins=joins, + ) + ] + + def test_empty_joins_introspects_no_referenced_table( + self, workspace: Path + ) -> None: + assert self._introspect(workspace, []) == ["id", "x"] + + def test_none_joins_still_falls_back(self, workspace: Path) -> None: + """`None` means joins were never generated — the fallback must stay.""" + assert self._introspect(workspace, None) == [ + "id", "x", "a__b.id", "a__b.label", + ] + + +class TestSanitizedNamesDoNotLeakIntoColumns: + """A dropped join must not reappear as `__`-bearing dotted columns.""" + + def test_no_dotted_columns_for_a_skipped_target(self, workspace: Path) -> None: + ds = _sqlite_ds( + workspace, + """ + CREATE TABLE a_b (id INTEGER PRIMARY KEY); + CREATE TABLE a__b (id INTEGER PRIMARY KEY, label TEXT); + CREATE TABLE refs_it ( + id INTEGER PRIMARY KEY, + x INTEGER REFERENCES a__b(id) + ); + """, + ) + models = {m.name: m for m in ingest_datasource(datasource=ds)} + refs_it = models["refs_it"] + assert refs_it.joins == [] + # The fallback used to fire on the now-empty join list and introspect + # the skipped object anyway, emitting `a__b.label` — a column name the + # SQL generator reads as a join path. + assert all("__" not in c.name for c in refs_it.columns) + assert all(not c.name.startswith("a_b.") for c in refs_it.columns) diff --git a/tests/test_join_cardinality_mirror.py b/tests/test_join_cardinality_mirror.py new file mode 100644 index 00000000..e119b5de --- /dev/null +++ b/tests/test_join_cardinality_mirror.py @@ -0,0 +1,124 @@ +"""The mirrored reverse INNER join must carry the inverted cardinality.""" + +import tempfile + +import pytest_asyncio + +from slayer.core.enums import DataType, JoinCardinality, JoinType +from slayer.core.models import Column, DatasourceConfig, ModelJoin, SlayerModel +from slayer.storage.join_sync import _mirror_inner_joins +from slayer.storage.yaml_storage import YAMLStorage + + +def _model(name: str, *, joins: list[ModelJoin] | None = None) -> SlayerModel: + return SlayerModel( + name=name, + sql_table=name, + data_source="test", + columns=[ + Column(name="id", sql="id", type=DataType.DOUBLE, primary_key=True), + Column(name="fk_id", sql="fk_id", type=DataType.DOUBLE), + ], + joins=joins or [], + ) + + +def _inner_join(target: str, cardinality: JoinCardinality | None) -> ModelJoin: + return ModelJoin( + target_model=target, + join_pairs=[["fk_id", "id"]], + join_type=JoinType.INNER, + cardinality=cardinality, + ) + + +@pytest_asyncio.fixture +async def raw_storage(): + with tempfile.TemporaryDirectory() as d: + storage = YAMLStorage(base_dir=d) + await storage.save_datasource( + DatasourceConfig(name="test", type="sqlite", database=":memory:") + ) + yield storage + + +async def _reverse_join(storage, source_name: str, from_model: str): + m = await storage.get_model(from_model) + return next((j for j in m.joins if j.target_model == source_name), None) + + +class TestMirrorInvertsCardinality: + async def test_many_to_one_reverses_to_one_to_many(self, raw_storage) -> None: + a = _model("a", joins=[_inner_join("b", JoinCardinality.MANY_TO_ONE)]) + b = _model("b") + await raw_storage.save_model(a) + await raw_storage.save_model(b) + + await _mirror_inner_joins(a, raw_storage) + + rev = await _reverse_join(raw_storage, "a", "b") + assert rev is not None + assert rev.cardinality is JoinCardinality.ONE_TO_MANY + + async def test_one_to_one_self_inverse(self, raw_storage) -> None: + a = _model("a", joins=[_inner_join("b", JoinCardinality.ONE_TO_ONE)]) + b = _model("b") + await raw_storage.save_model(a) + await raw_storage.save_model(b) + + await _mirror_inner_joins(a, raw_storage) + + rev = await _reverse_join(raw_storage, "a", "b") + assert rev.cardinality is JoinCardinality.ONE_TO_ONE + + async def test_many_to_many_self_inverse(self, raw_storage) -> None: + a = _model("a", joins=[_inner_join("b", JoinCardinality.MANY_TO_MANY)]) + b = _model("b") + await raw_storage.save_model(a) + await raw_storage.save_model(b) + + await _mirror_inner_joins(a, raw_storage) + + rev = await _reverse_join(raw_storage, "a", "b") + assert rev.cardinality is JoinCardinality.MANY_TO_MANY + + async def test_none_cardinality_reverse_stays_none(self, raw_storage) -> None: + a = _model("a", joins=[_inner_join("b", None)]) + b = _model("b") + await raw_storage.save_model(a) + await raw_storage.save_model(b) + + await _mirror_inner_joins(a, raw_storage) + + rev = await _reverse_join(raw_storage, "a", "b") + assert rev is not None + assert rev.cardinality is None + + async def test_existing_reverse_reconciled_when_only_cardinality_changes( + self, raw_storage + ) -> None: + # b already has a reverse join to a, but with a stale/absent cardinality. + a = _model("a", joins=[_inner_join("b", JoinCardinality.MANY_TO_ONE)]) + b = _model( + "b", + joins=[ + ModelJoin( + target_model="a", + join_pairs=[["id", "fk_id"]], + join_type=JoinType.INNER, + cardinality=None, + ) + ], + ) + await raw_storage.save_model(a) + await raw_storage.save_model(b) + + await _mirror_inner_joins(a, raw_storage) + + rev = await _reverse_join(raw_storage, "a", "b") + # Reverse of many_to_one is one_to_many — reconciled even though the + # join_pairs were already correct. + assert rev.cardinality is JoinCardinality.ONE_TO_MANY + # Not duplicated. + b_reloaded = await raw_storage.get_model("b") + assert sum(1 for j in b_reloaded.joins if j.target_model == "a") == 1 diff --git a/tests/test_join_cardinality_models.py b/tests/test_join_cardinality_models.py new file mode 100644 index 00000000..5a54f3e5 --- /dev/null +++ b/tests/test_join_cardinality_models.py @@ -0,0 +1,232 @@ +"""Data-model coverage for ``ModelJoin.cardinality`` and ``Column.unique``.""" + +import tempfile + +from slayer.core.enums import DataType, JoinCardinality, JoinType, invert_cardinality +from slayer.core.models import Column, ModelJoin, SlayerModel +from slayer.storage.sqlite_storage import SQLiteStorage +from slayer.storage.yaml_storage import YAMLStorage + + +# --------------------------------------------------------------------------- +# JoinCardinality enum +# --------------------------------------------------------------------------- + + +def test_join_cardinality_enum_values() -> None: + # LookML-style string values, source->target reading. + assert JoinCardinality.ONE_TO_ONE == "one_to_one" + assert JoinCardinality.ONE_TO_MANY == "one_to_many" + assert JoinCardinality.MANY_TO_ONE == "many_to_one" + assert JoinCardinality.MANY_TO_MANY == "many_to_many" + assert {c.value for c in JoinCardinality} == { + "one_to_one", + "one_to_many", + "many_to_one", + "many_to_many", + } + + +def test_join_cardinality_from_string() -> None: + assert JoinCardinality("many_to_one") is JoinCardinality.MANY_TO_ONE + + +# --------------------------------------------------------------------------- +# invert_cardinality +# --------------------------------------------------------------------------- + + +def test_invert_cardinality_swaps_directional() -> None: + assert invert_cardinality(JoinCardinality.MANY_TO_ONE) is JoinCardinality.ONE_TO_MANY + assert invert_cardinality(JoinCardinality.ONE_TO_MANY) is JoinCardinality.MANY_TO_ONE + + +def test_invert_cardinality_self_inverse_symmetric() -> None: + assert invert_cardinality(JoinCardinality.ONE_TO_ONE) is JoinCardinality.ONE_TO_ONE + assert invert_cardinality(JoinCardinality.MANY_TO_MANY) is JoinCardinality.MANY_TO_MANY + + +def test_invert_cardinality_none_passthrough() -> None: + assert invert_cardinality(None) is None + + +def test_invert_cardinality_is_involutive() -> None: + for c in JoinCardinality: + assert invert_cardinality(invert_cardinality(c)) is c + + +# --------------------------------------------------------------------------- +# ModelJoin.cardinality +# --------------------------------------------------------------------------- + + +def test_modeljoin_cardinality_defaults_none() -> None: + j = ModelJoin(target_model="customers", join_pairs=[["customer_id", "id"]]) + assert j.cardinality is None + + +def test_modeljoin_accepts_cardinality_enum_and_string() -> None: + j1 = ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality=JoinCardinality.MANY_TO_ONE, + ) + assert j1.cardinality is JoinCardinality.MANY_TO_ONE + + j2 = ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality="one_to_one", + ) + assert j2.cardinality is JoinCardinality.ONE_TO_ONE + + +def test_modeljoin_cardinality_serializes() -> None: + j = ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality=JoinCardinality.MANY_TO_ONE, + ) + dumped = j.model_dump() + assert dumped["cardinality"] == "many_to_one" + # Round-trips back into an equivalent object. + assert ModelJoin.model_validate(dumped).cardinality is JoinCardinality.MANY_TO_ONE + + +def test_modeljoin_cardinality_orthogonal_to_join_type() -> None: + # LEFT join can still carry many_to_one; the two axes are independent. + j = ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + join_type=JoinType.LEFT, + cardinality=JoinCardinality.MANY_TO_ONE, + ) + assert j.join_type is JoinType.LEFT + assert j.cardinality is JoinCardinality.MANY_TO_ONE + + +# --------------------------------------------------------------------------- +# Column.unique +# --------------------------------------------------------------------------- + + +def test_column_unique_defaults_false() -> None: + c = Column(name="id", type=DataType.INT) + assert c.unique is False + + +def test_column_accepts_unique() -> None: + c = Column(name="email", type=DataType.TEXT, unique=True) + assert c.unique is True + + +def test_column_unique_serializes() -> None: + c = Column(name="email", type=DataType.TEXT, unique=True) + assert c.model_dump()["unique"] is True + assert Column.model_validate(c.model_dump()).unique is True + + +# --------------------------------------------------------------------------- +# Back-compat: old data without the new fields validates unchanged (no bump) +# --------------------------------------------------------------------------- + + +def test_old_join_without_cardinality_validates() -> None: + j = ModelJoin.model_validate({"target_model": "c", "join_pairs": [["a", "b"]]}) + assert j.cardinality is None + + +def test_old_column_without_unique_validates() -> None: + c = Column.model_validate({"name": "id", "type": "INT"}) + assert c.unique is False + + +def test_current_version_data_without_the_new_fields_needs_no_migration() -> None: + """cardinality / unique are additive — they default in with no version change. + + Payload is stamped at the CURRENT version, so nothing migrates: if either + field ever needed a migration step, the version would have to move and this + assertion would fail. (Asserting a literal, or the default, against a + migrated v7 payload proves nothing — both pass after any unrelated bump.) + """ + current_version = SlayerModel.model_fields["version"].default + m = SlayerModel.model_validate( + { + "version": current_version, + "name": "orders", + "sql_table": "orders", + "data_source": "testds", + "columns": [{"name": "customer_id", "type": "INT"}], + "joins": [ + {"target_model": "customers", "join_pairs": [["customer_id", "id"]]} + ], + } + ) + assert m.version == current_version + assert m.joins[0].cardinality is None + assert m.columns[0].unique is False + + +def test_pre_existing_v7_data_still_validates() -> None: + """Older payloads migrate up and the new fields default in.""" + m = SlayerModel.model_validate( + { + "version": 7, + "name": "orders", + "sql_table": "orders", + "data_source": "testds", + "columns": [{"name": "customer_id", "type": "INT"}], + "joins": [ + {"target_model": "customers", "join_pairs": [["customer_id", "id"]]} + ], + } + ) + assert m.joins[0].cardinality is None + assert m.columns[0].unique is False + + +# --------------------------------------------------------------------------- +# Storage round-trips (both backends persist the new fields) +# --------------------------------------------------------------------------- + + +def _model_with_cardinality() -> SlayerModel: + return SlayerModel( + name="orders", + sql_table="orders", + data_source="testds", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="email", type=DataType.TEXT, unique=True), + Column(name="customer_id", type=DataType.INT), + ], + joins=[ + ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality=JoinCardinality.MANY_TO_ONE, + ) + ], + ) + + +async def test_yaml_roundtrip_preserves_cardinality_and_unique() -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_model(_model_with_cardinality()) + loaded = await storage.get_model("orders", data_source="testds") + assert loaded is not None + assert loaded.joins[0].cardinality is JoinCardinality.MANY_TO_ONE + unique_col = next(c for c in loaded.columns if c.name == "email") + assert unique_col.unique is True + + +async def test_sqlite_roundtrip_preserves_cardinality_and_unique() -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = SQLiteStorage(db_path=f"{tmp}/s.db") + await storage.save_model(_model_with_cardinality()) + loaded = await storage.get_model("orders", data_source="testds") + assert loaded is not None + assert loaded.joins[0].cardinality is JoinCardinality.MANY_TO_ONE + unique_col = next(c for c in loaded.columns if c.name == "email") + assert unique_col.unique is True diff --git a/tests/test_join_cardinality_producers.py b/tests/test_join_cardinality_producers.py new file mode 100644 index 00000000..8d4cae8b --- /dev/null +++ b/tests/test_join_cardinality_producers.py @@ -0,0 +1,312 @@ +"""dbt / OSI / facade producers set and carry join cardinality.""" + +from pathlib import Path + +import pytest +import sqlalchemy as sa + +from slayer.core.enums import DataType, JoinCardinality +from slayer.core.models import Column, ModelJoin, SlayerModel +from slayer.dbt.converter import DbtToSlayerConverter +from slayer.dbt.entities import EntityRegistry +from slayer.dbt.models import DbtEntity, DbtProject, DbtSemanticModel +from slayer.facade.catalog import FacadeJoin, build_catalog, _facade_join_from +from slayer.facade.translator import translate +from slayer.osi.converter import OsiToSlayerConverter +from slayer.osi.parser import parse_osi_path + +FIXTURES = Path(__file__).parent / "fixtures" / "osi" + +_OSI_SCHEMA = [ + "CREATE TABLE orders (order_id INTEGER PRIMARY KEY, customer_id INTEGER, " + "product_id INTEGER, amount REAL, quantity INTEGER, ordered_at DATE, status TEXT)", + "CREATE TABLE customers (customer_id INTEGER PRIMARY KEY, region_id INTEGER, " + "name TEXT, segment TEXT)", + "CREATE TABLE products (product_id INTEGER PRIMARY KEY, category TEXT, price REAL)", + "CREATE TABLE regions (region_id INTEGER PRIMARY KEY, name TEXT, population INTEGER)", + "CREATE TABLE ckey_parent (k1 INTEGER, k2 INTEGER, label TEXT, PRIMARY KEY (k1, k2))", + "CREATE TABLE ckey_child (k1 INTEGER, k2 INTEGER, v REAL)", +] + + +def _foreign_primary_project() -> DbtProject: + return DbtProject( + semantic_models=[ + DbtSemanticModel( + name="orders", + model="orders", + entities=[ + DbtEntity(name="order_id", type="primary", expr="id"), + DbtEntity(name="customer_id", type="foreign", expr="customer_id"), + ], + ), + DbtSemanticModel( + name="customers", + model="customers", + entities=[DbtEntity(name="customer_id", type="primary", expr="id")], + ), + ] + ) + + +# --------------------------------------------------------------------------- +# dbt entity-registry level +# --------------------------------------------------------------------------- + + +class TestDbtEntities: + def test_foreign_to_primary_is_many_to_one(self) -> None: + orders = DbtSemanticModel( + name="orders", + entities=[ + DbtEntity(name="order_id", type="primary", expr="id"), + DbtEntity(name="customer_id", type="foreign", expr="customer_id"), + ], + ) + customers = DbtSemanticModel( + name="customers", + entities=[DbtEntity(name="customer_id", type="primary", expr="id")], + ) + reg = EntityRegistry() + reg.build([orders, customers]) + joins = reg.resolve_joins_for_model(orders) + assert joins[0].cardinality is JoinCardinality.MANY_TO_ONE + + def test_peer_shared_unique_is_one_to_one(self) -> None: + a = DbtSemanticModel( + name="a", entities=[DbtEntity(name="shared", type="unique")] + ) + b = DbtSemanticModel( + name="b", entities=[DbtEntity(name="shared", type="unique")] + ) + reg = EntityRegistry() + reg.build([a, b]) + joins = reg.resolve_joins_for_model(a) + peer = next(j for j in joins if j.target_model == "b") + assert peer.cardinality is JoinCardinality.ONE_TO_ONE + + +# --------------------------------------------------------------------------- +# dbt converter (with the in-memory inner-join mirror) +# --------------------------------------------------------------------------- + + +class TestDbtConverterMirror: + def test_forward_many_to_one(self) -> None: + result = DbtToSlayerConverter( + project=_foreign_primary_project(), data_source="test_db" + ).convert() + orders = next(m for m in result.models if m.name == "orders") + fwd = next(j for j in orders.joins if j.target_model == "customers") + assert fwd.cardinality is JoinCardinality.MANY_TO_ONE + + def test_reverse_mirror_inverts_to_one_to_many(self) -> None: + result = DbtToSlayerConverter( + project=_foreign_primary_project(), data_source="test_db" + ).convert() + customers = next(m for m in result.models if m.name == "customers") + rev = next(j for j in customers.joins if j.target_model == "orders") + # Reverse of many_to_one is one_to_many. + assert rev.cardinality is JoinCardinality.ONE_TO_MANY + + def test_peer_mirror_stays_one_to_one(self) -> None: + project = DbtProject( + semantic_models=[ + DbtSemanticModel( + name="claim", + model="claim", + entities=[DbtEntity(name="claim_identifier", type="primary")], + ), + DbtSemanticModel( + name="claim_coverage", + model="claim_coverage", + entities=[DbtEntity(name="claim_identifier", type="primary")], + ), + ] + ) + result = DbtToSlayerConverter(project=project, data_source="test").convert() + claim = next(m for m in result.models if m.name == "claim") + cov = next(m for m in result.models if m.name == "claim_coverage") + assert ( + next(j for j in claim.joins if j.target_model == "claim_coverage").cardinality + is JoinCardinality.ONE_TO_ONE + ) + assert ( + next(j for j in cov.joins if j.target_model == "claim").cardinality + is JoinCardinality.ONE_TO_ONE + ) + + +# --------------------------------------------------------------------------- +# OSI +# --------------------------------------------------------------------------- + + +@pytest.fixture +def osi_engine(tmp_path: Path) -> sa.Engine: + engine = sa.create_engine(f"sqlite:///{tmp_path}/shop.db") + with engine.connect() as conn: + for ddl in _OSI_SCHEMA: + conn.execute(sa.text(ddl)) + conn.commit() + return engine + + +class TestOsi: + def test_relationship_is_many_to_one(self, osi_engine: sa.Engine) -> None: + doc = parse_osi_path(FIXTURES / "shop.yaml")[0] + result = OsiToSlayerConverter( + documents=[doc], data_source="testds", sa_engine=osi_engine + ).convert() + orders = {m.name: m for m in result.models}["orders"] + cust_join = next(j for j in orders.joins if j.target_model == "customers") + assert cust_join.cardinality is JoinCardinality.MANY_TO_ONE + + +# --------------------------------------------------------------------------- +# Facade +# --------------------------------------------------------------------------- + + +class TestFacade: + def test_facade_join_model_accepts_cardinality(self) -> None: + fj = FacadeJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality=JoinCardinality.ONE_TO_ONE, + ) + assert fj.cardinality is JoinCardinality.ONE_TO_ONE + + def test_facade_join_from_carries_cardinality(self) -> None: + j = ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality=JoinCardinality.MANY_TO_ONE, + ) + fj = _facade_join_from(join=j) + assert fj.cardinality is JoinCardinality.MANY_TO_ONE + + +def _dyn_catalog(): + """orders (no configured join) + stores — forces the dynamic-join path.""" + orders = SlayerModel( + name="orders", + data_source="jaffle", + sql_table="orders", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="store_id", type=DataType.INT), + Column(name="revenue", type=DataType.DOUBLE), + ], + joins=[], + ) + stores = SlayerModel( + name="stores", + data_source="jaffle", + sql_table="stores", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="name", type=DataType.TEXT), + Column(name="tax_rate", type=DataType.DOUBLE), + ], + ) + return build_catalog(models_by_datasource={"jaffle": [orders, stores]}) + + +def _metabase_join_sql(on_clause: str) -> str: + return ( + 'SELECT "Stores"."name" AS "Stores__name" ' + 'FROM "public"."orders" ' + 'LEFT JOIN (SELECT "public"."stores"."id" AS "id", ' + '"public"."stores"."name" AS "name", ' + '"public"."stores"."tax_rate" AS "tax_rate" ' + 'FROM "public"."stores") AS "Stores" ' + f"ON {on_clause}" + ) + + +class TestFacadeDynamicJoin: + def test_dynamic_join_many_to_one_when_target_unique(self) -> None: + # ON joins to stores.id (the PK) -> target is unique -> many_to_one. + sql = _metabase_join_sql('"public"."orders"."store_id" = "Stores"."id"') + result = translate(sql=sql, catalog=_dyn_catalog(), dialect="postgres") + ext = result.query.source_model + assert ext.joins[0].cardinality is JoinCardinality.MANY_TO_ONE + + def test_dynamic_join_none_when_target_not_unique(self) -> None: + # ON joins to stores.tax_rate (not PK/unique) -> undetermined -> None. + sql = _metabase_join_sql('"public"."orders"."store_id" = "Stores"."tax_rate"') + result = translate(sql=sql, catalog=_dyn_catalog(), dialect="postgres") + ext = result.query.source_model + assert ext.joins[0].cardinality is None + + def test_dynamic_join_none_for_composite_pk_member(self) -> None: + """Joining ONE member of a composite PK does not make the target unique.""" + orders = SlayerModel( + name="orders", + data_source="jaffle", + sql_table="orders", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="store_id", type=DataType.INT), + ], + joins=[], + ) + # Composite PK (id, region) — neither column is unique on its own. + stores = SlayerModel( + name="stores", + data_source="jaffle", + sql_table="stores", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="region", type=DataType.TEXT, primary_key=True), + Column(name="name", type=DataType.TEXT), + ], + ) + catalog = build_catalog(models_by_datasource={"jaffle": [orders, stores]}) + sql = ( + 'SELECT "Stores"."name" AS "Stores__name" ' + 'FROM "public"."orders" ' + 'LEFT JOIN (SELECT "public"."stores"."id" AS "id", ' + '"public"."stores"."name" AS "name" ' + 'FROM "public"."stores") AS "Stores" ' + 'ON "public"."orders"."store_id" = "Stores"."id"' + ) + result = translate(sql=sql, catalog=catalog, dialect="postgres") + ext = result.query.source_model + assert ext.joins[0].cardinality is None + + def test_dynamic_join_many_to_one_for_non_pk_unique_target(self) -> None: + """A non-PK column flagged ``unique`` is a solo uniqueness claim.""" + orders = SlayerModel( + name="orders", + data_source="jaffle", + sql_table="orders", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="store_code", type=DataType.TEXT), + ], + joins=[], + ) + stores = SlayerModel( + name="stores", + data_source="jaffle", + sql_table="stores", + columns=[ + Column(name="id", type=DataType.INT, primary_key=True), + Column(name="code", type=DataType.TEXT, unique=True), + Column(name="name", type=DataType.TEXT), + ], + ) + catalog = build_catalog(models_by_datasource={"jaffle": [orders, stores]}) + sql = ( + 'SELECT "Stores"."name" AS "Stores__name" ' + 'FROM "public"."orders" ' + 'LEFT JOIN (SELECT "public"."stores"."code" AS "code", ' + '"public"."stores"."name" AS "name" ' + 'FROM "public"."stores") AS "Stores" ' + 'ON "public"."orders"."store_code" = "Stores"."code"' + ) + result = translate(sql=sql, catalog=catalog, dialect="postgres") + ext = result.query.source_model + assert ext.joins[0].cardinality is JoinCardinality.MANY_TO_ONE diff --git a/tests/test_join_cardinality_surfacing.py b/tests/test_join_cardinality_surfacing.py new file mode 100644 index 00000000..624a683a --- /dev/null +++ b/tests/test_join_cardinality_surfacing.py @@ -0,0 +1,203 @@ +"""Cardinality/unique surfacing in inspect, search graph, edit_model — not in embeddings.""" + +from __future__ import annotations + +import json +import tempfile + +import pytest + +from slayer.core.enums import DataType, JoinCardinality +from slayer.core.models import Column, DatasourceConfig, ModelJoin, SlayerModel +from slayer.inspect.model_render import render_model_inspection +from slayer.mcp.server import create_mcp_server +from slayer.search import graph as search_graph +from slayer.search.render import render_column_text, render_model_text +from slayer.storage.yaml_storage import YAMLStorage + + +def _orders_model(*, cardinality: JoinCardinality | None, email_unique: bool) -> SlayerModel: + return SlayerModel( + name="orders", + sql_table="orders", + data_source="ds", + columns=[ + Column(name="id", sql="id", type=DataType.INT, primary_key=True), + Column(name="email", sql="email", type=DataType.TEXT, unique=email_unique), + Column(name="customer_id", sql="customer_id", type=DataType.INT), + ], + joins=[ + ModelJoin( + target_model="customers", + join_pairs=[["customer_id", "id"]], + cardinality=cardinality, + ) + ], + ) + + +def _customers_model() -> SlayerModel: + return SlayerModel( + name="customers", + sql_table="customers", + data_source="ds", + columns=[Column(name="id", sql="id", type=DataType.INT, primary_key=True)], + ) + + +# --------------------------------------------------------------------------- +# render_model_inspection +# --------------------------------------------------------------------------- + + +class TestModelRender: + async def test_json_joins_carry_cardinality(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_datasource( + DatasourceConfig(name="ds", type="sqlite", database=":memory:") + ) + model = _orders_model( + cardinality=JoinCardinality.MANY_TO_ONE, email_unique=True + ) + out = await render_model_inspection( + model=model, storage=storage, engine=None, format="json", compact=False + ) + payload = json.loads(out) + join = payload["joins"][0] + assert join["cardinality"] == "many_to_one" + + async def test_json_columns_carry_unique(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_datasource( + DatasourceConfig(name="ds", type="sqlite", database=":memory:") + ) + model = _orders_model( + cardinality=JoinCardinality.MANY_TO_ONE, email_unique=True + ) + out = await render_model_inspection( + model=model, storage=storage, engine=None, format="json", compact=False + ) + payload = json.loads(out) + email = next(c for c in payload["columns"] if c["name"] == "email") + assert email["unique"] is True + + async def test_markdown_shows_cardinality(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_datasource( + DatasourceConfig(name="ds", type="sqlite", database=":memory:") + ) + model = _orders_model( + cardinality=JoinCardinality.MANY_TO_ONE, email_unique=True + ) + md = await render_model_inspection( + model=model, storage=storage, engine=None, format="markdown", compact=False + ) + assert "many_to_one" in md + + async def test_markdown_shows_unique_column(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_datasource( + DatasourceConfig(name="ds", type="sqlite", database=":memory:") + ) + model = _orders_model( + cardinality=JoinCardinality.MANY_TO_ONE, email_unique=True + ) + md = await render_model_inspection( + model=model, storage=storage, engine=None, format="markdown", compact=False + ) + # The columns table gains a `unique` column. + assert "unique" in md.lower() + + +# --------------------------------------------------------------------------- +# search-graph JOINS edge property +# --------------------------------------------------------------------------- + + +@pytest.mark.skipif( + not search_graph.is_available(), reason="graph backend (ladybug) not installed" +) +class TestSearchGraphEdge: + async def test_joins_edge_has_cardinality_property(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_datasource( + DatasourceConfig(name="ds", type="sqlite", database=":memory:") + ) + await storage.save_model(_customers_model()) + await storage.save_model( + _orders_model(cardinality=JoinCardinality.MANY_TO_ONE, email_unique=False) + ) + search_graph.clear_cache() + ids = await search_graph.get_filtered_ids( + "MATCH (m:Model {id: 'ds.orders'})-[r:JOINS]->(t:Model) " + "WHERE r.cardinality = 'many_to_one' RETURN t.id AS id", + storage, + ) + assert ids == {"ds.customers"} + + +# --------------------------------------------------------------------------- +# edit_model round-trip (fields flow via _upsert_entity.model_validate) +# --------------------------------------------------------------------------- + + +class TestEditModelRoundTrip: + async def test_edit_model_accepts_cardinality_and_unique(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + storage = YAMLStorage(base_dir=tmp) + await storage.save_datasource( + DatasourceConfig(name="ds", type="sqlite", database=":memory:") + ) + await storage.save_model( + _orders_model(cardinality=None, email_unique=False) + ) + server = create_mcp_server(storage=storage) + + await server.call_tool( + name="edit_model", + arguments={ + "model_name": "orders", + "data_source": "ds", + "columns": [{"name": "email", "unique": True}], + "joins": [ + { + "target_model": "customers", + "join_pairs": [["customer_id", "id"]], + "cardinality": "many_to_one", + } + ], + }, + ) + + reloaded = await storage.get_model("orders", data_source="ds") + join = next(j for j in reloaded.joins if j.target_model == "customers") + assert join.cardinality is JoinCardinality.MANY_TO_ONE + email = next(c for c in reloaded.columns if c.name == "email") + assert email.unique is True + + +# --------------------------------------------------------------------------- +# No embedding churn — new fields excluded from the embedded corpus text +# --------------------------------------------------------------------------- + + +class TestNoEmbeddingChurn: + def test_cardinality_not_in_model_embedding_text(self) -> None: + plain = _orders_model(cardinality=None, email_unique=False) + carded = _orders_model( + cardinality=JoinCardinality.MANY_TO_ONE, email_unique=False + ) + assert render_model_text(model=carded) == render_model_text(model=plain) + + def test_unique_not_in_column_embedding_text(self) -> None: + model = _orders_model(cardinality=None, email_unique=False) + plain_col = Column(name="email", sql="email", type=DataType.TEXT, unique=False) + uniq_col = Column(name="email", sql="email", type=DataType.TEXT, unique=True) + assert render_column_text(model=model, column=uniq_col) == render_column_text( + model=model, column=plain_col + )