From e2c721bd411ec3424cf691b30be7205d07c32dbd Mon Sep 17 00:00:00 2001 From: Egor Kraev Date: Wed, 24 Jun 2026 17:42:21 +0200 Subject: [PATCH 01/17] Strengthen MetricFlow ingestion into SLayer (DEV-1595) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the dbt/MetricFlow importer either represent every legal DSI construct exactly or fail it cleanly into a structured report — no silent drops, no approximate/lossy SQL. count_distinct_approx (dialect-aware aggregation): - enums: builtin + eligible on every type and PK columns + aliases - SqlDialect.build_approx_count_distinct: native function per dialect, exact COUNT(DISTINCT) fallback (Postgres/SQLite/MySQL) - generator dispatch composing with the filter-CASE wrapper Importer correctness + represent-exactly: - percentile p= (+ discrete/approx flag clean-fail), ratio nullif guard - sum_boolean -> CASE INT column, offset_window -> time_shift - metric/per-input filter push-down with leaf dedup + cross-model reachability; string-or-list filter normalization Parser completeness: cumulative/conversion/aggregation type-params, time-window parsing, measure str-or-object, join_to_timespine/fill_nulls_with, config.meta. extra="ignore" kept. Clean-fail routing + report: ConversionWarning category/severity/suggestion, render_report(), meta stash of dropped constructs, target_dialect percentile caveat, CLI grouped report + tally. Docs updated across dbt import, database-support, aggregation, model, and skill references. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/skills/slayer-models.md | 2 +- .claude/skills/slayer-query.md | 2 +- CLAUDE.md | 2 +- docs/concepts/models.md | 10 +- docs/concepts/terminology.md | 2 +- docs/database-support.md | 22 +- docs/dbt/dbt_import.md | 60 +- docs/dbt/slayer_vs_dbt.md | 10 +- docs/examples/07_aggregations/aggregations.md | 14 + slayer/cli.py | 15 +- slayer/core/enums.py | 17 +- slayer/dbt/converter.py | 1110 ++++++++++++----- slayer/dbt/models.py | 209 +++- slayer/sql/dialects/_tier2.py | 63 +- slayer/sql/dialects/base.py | 17 + slayer/sql/dialects/bigquery.py | 9 + slayer/sql/dialects/clickhouse.py | 9 + slayer/sql/dialects/duckdb.py | 13 +- slayer/sql/dialects/snowflake.py | 12 +- slayer/sql/dialects/tsql.py | 14 + slayer/sql/generator.py | 19 + tests/dialects/test_count_distinct_approx.py | 142 +++ tests/integration/test_integration_duckdb.py | 24 + .../test_count_distinct_approx_generation.py | 120 ++ tests/test_dbt_converter.py | 8 +- tests/test_dbt_metricflow_strengthen.py | 874 +++++++++++++ tests/test_dbt_parser.py | 4 +- tests/test_enums.py | 6 +- tests/test_filtered_count_forms.py | 57 + 29 files changed, 2527 insertions(+), 339 deletions(-) create mode 100644 tests/dialects/test_count_distinct_approx.py create mode 100644 tests/test_count_distinct_approx_generation.py create mode 100644 tests/test_dbt_metricflow_strengthen.py create mode 100644 tests/test_filtered_count_forms.py diff --git a/.claude/skills/slayer-models.md b/.claude/skills/slayer-models.md index 32fc7991..6b23bc33 100644 --- a/.claude/skills/slayer-models.md +++ b/.claude/skills/slayer-models.md @@ -43,7 +43,7 @@ measures: formula: "amount:sum / *:count" ``` -Aggregation is specified at query time with **colon syntax**: `"amount:sum"`, `"amount:avg"`, `"*:count"`. A bare-name reference like `{"formula": "aov"}` resolves to the saved `ModelMeasure` formula on the model. Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. The two-column ones (`corr`, `covar_samp`, `covar_pop`) take the second column as a named param: `price:corr(other=quantity)`. +Aggregation is specified at query time with **colon syntax**: `"amount:sum"`, `"amount:avg"`, `"*:count"`. A bare-name reference like `{"formula": "aov"}` resolves to the saved `ModelMeasure` formula on the model. Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `count_distinct_approx`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. `count_distinct_approx` is dialect-aware (native approximate-distinct where available, exact `COUNT(DISTINCT)` fallback otherwise). The two-column ones (`corr`, `covar_samp`, `covar_pop`) take the second column as a named param: `price:corr(other=quantity)`. ## Data Types diff --git a/.claude/skills/slayer-query.md b/.claude/skills/slayer-query.md index e42cbb70..9f4869e3 100644 --- a/.claude/skills/slayer-query.md +++ b/.claude/skills/slayer-query.md @@ -46,7 +46,7 @@ Each entry in `measures` is either a bare formula string or a `{"formula": ..., ] ``` -Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. Two-column `corr`/`covar_samp`/`covar_pop` take the second column as a named param: `price:corr(other=quantity)`. `sum` and `avg` accept an optional trailing-window: `revenue:sum(window='30d')`. +Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `count_distinct_approx`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. `count_distinct_approx` is dialect-aware (native approximate-distinct where available, exact `COUNT(DISTINCT)` fallback otherwise). Two-column `corr`/`covar_samp`/`covar_pop` take the second column as a named param: `price:corr(other=quantity)`. `sum` and `avg` accept an optional trailing-window: `revenue:sum(window='30d')`. `*:count` is always available — no column definition needed. `col:count` counts non-nulls. diff --git a/CLAUDE.md b/CLAUDE.md index 3ae94b2a..8bfd3a9b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,7 +59,7 @@ poetry run ruff check slayer/ tests/ - **Run-by-name execution**: `engine.execute(str, variables=..., dry_run=..., explain=...)` and `execute_sync(str, ...)` run the stored backing query for a query-backed model. Errors surface as `Model '' not found` or `Model '' is not query-backed; pass a SlayerQuery with source_model=''.`. Variable precedence (highest first): runtime kwarg > stage > outer query > model defaults. The `variables=` kwarg works uniformly for str, dict, SlayerQuery, and list inputs. Runtime kwargs are merged into the available variable set; extra keys not referenced by any `{var}` placeholder simply remain unused. `dry_run`/`explain` are engine kwargs (not query fields) and apply to every input shape. Surfaced via REST `POST /query` with `{"name": "...", "variables": {...}}`, MCP `query` tool with `variables=`, CLI `slayer query --variables k=v`. - **Unified columns** (v2): `SlayerModel.columns: List[Column]` replaces v1's separate `dimensions` and `measures`. A `Column` carries name, sql, type (`DataType`), `primary_key`, `description`, `label`, `hidden`, `format`, `allowed_aggregations` (whitelist), `filter` (CASE-WHEN at aggregation time), `meta`. What a column is "used as" (group-by dim vs aggregation source) is decided per query. - **Measures are named formulas**: `SlayerModel.measures: List[ModelMeasure]` is a library of saved formulas of shape `{formula, name, label, description}`. Same shape as the inline `SlayerQuery.measures` entries. Queries can reference them by bare name (`{formula: "aov"}`) or expand them inline. -- **Aggregations are query-time**: specified via **colon syntax** in formulas — `"revenue:sum"`, `"*:count"`, `"price:weighted_avg(weight=quantity)"`, `"price:corr(other=quantity)"`. Built-in aggregations: sum, avg, min, max, count, count_distinct, first, last, weighted_avg, median, percentile, stddev_samp, stddev_pop, var_samp, var_pop, corr, covar_samp, covar_pop. Custom aggregations defined at model level in `aggregations` list. +- **Aggregations are query-time**: specified via **colon syntax** in formulas — `"revenue:sum"`, `"*:count"`, `"price:weighted_avg(weight=quantity)"`, `"price:corr(other=quantity)"`. Built-in aggregations: sum, avg, min, max, count, count_distinct, count_distinct_approx, first, last, weighted_avg, median, percentile, stddev_samp, stddev_pop, var_samp, var_pop, corr, covar_samp, covar_pop. Custom aggregations defined at model level in `aggregations` list. **`count_distinct_approx`** (DEV-1595, aliases `approx_count_distinct` / `countdistinctapprox`) is dialect-aware: it emits the DB-native approximate-distinct (`approx_count_distinct` on DuckDB/Spark/Databricks, `uniq` on ClickHouse, `APPROX_COUNT_DISTINCT` on BigQuery/Snowflake/T-SQL/Oracle, `approx_distinct` on Trino/Presto, `APPROXIMATE COUNT(DISTINCT)` on Redshift) and falls back to exact `COUNT(DISTINCT)` where there's no native function (Postgres/SQLite/MySQL) — exact is *more* accurate, consistent with the no-approximate-SQL rule. Per-dialect mapping lives in `SqlDialect.build_approx_count_distinct` (`slayer/sql/dialects/`); eligible on every type and on PK columns, like `count_distinct`. - **`*:count`** for COUNT(*) — `*` means "all rows", `count` is just a regular aggregation. `col:count` = COUNT(col) for non-nulls. - Columns can have `allowed_aggregations` whitelist — validated at model creation and query time. Primary-key columns are always restricted to `count`/`count_distinct` regardless of type. Default eligibility per data type lives in `slayer/core/enums.py:DEFAULT_AGGREGATIONS_BY_TYPE`. - Auto-ingestion emits one `Column` per non-joined column. PK columns get `primary_key=True`. Columns named "count" rename to "count_col" to avoid clashing with `*:count`. diff --git a/docs/concepts/models.md b/docs/concepts/models.md index c76db918..4210e6f8 100644 --- a/docs/concepts/models.md +++ b/docs/concepts/models.md @@ -89,12 +89,12 @@ A column with no explicit `allowed_aggregations` whitelist gets a default set ba | Type | Default eligible aggregations | |------|-------------------------------| -| `number` | sum, avg, min, max, count, count_distinct, median, weighted_avg, percentile, first, last, stddev_samp, stddev_pop, var_samp, var_pop, corr, covar_samp, covar_pop | -| `string` | count, count_distinct, first, last, min, max | -| `boolean` | count, count_distinct, sum, min, max, first, last | -| `date` / `time` | count, count_distinct, first, last, min, max | +| `number` | sum, avg, min, max, count, count_distinct, count_distinct_approx, median, weighted_avg, percentile, first, last, stddev_samp, stddev_pop, var_samp, var_pop, corr, covar_samp, covar_pop | +| `string` | count, count_distinct, count_distinct_approx, first, last, min, max | +| `boolean` | count, count_distinct, count_distinct_approx, sum, min, max, first, last | +| `date` / `time` | count, count_distinct, count_distinct_approx, first, last, min, max | -Primary-key columns are always restricted to `count` / `count_distinct` regardless of type. When `allowed_aggregations` is set, every entry must already be eligible under the type-default map (or be a custom aggregation defined on this model); violations are caught at model construction time, so query-time validation is a single membership check. +`count_distinct_approx` is dialect-aware: it emits the database-native approximate-distinct function where one exists and falls back to an exact `COUNT(DISTINCT)` where it does not (Postgres / SQLite / MySQL). Primary-key columns are always restricted to `count` / `count_distinct` / `count_distinct_approx` regardless of type. When `allowed_aggregations` is set, every entry must already be eligible under the type-default map (or be a custom aggregation defined on this model); violations are caught at model construction time, so query-time validation is a single membership check. ### Filtered columns diff --git a/docs/concepts/terminology.md b/docs/concepts/terminology.md index aeda0eed..0e01dd0d 100644 --- a/docs/concepts/terminology.md +++ b/docs/concepts/terminology.md @@ -14,7 +14,7 @@ Key terms used throughout SLayer documentation and code. **Measure (named formula)** — A saved formula stored on a model (`SlayerModel.measures: List[ModelMeasure]`). Shape `{formula, name, label, description}` — same as a query's inline `measures` entry. Queries reference saved measures by bare name in any formula context (`{"formula": "aov"}`). -**Aggregation** — How a column is rolled up. Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. Custom aggregations can be defined at model level. Applied at query time via colon syntax: `revenue:sum`, `*:count`, `price:weighted_avg(weight=quantity)`, `price:corr(other=quantity)`. +**Aggregation** — How a column is rolled up. Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `count_distinct_approx`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. Custom aggregations can be defined at model level. Applied at query time via colon syntax: `revenue:sum`, `*:count`, `price:weighted_avg(weight=quantity)`, `price:corr(other=quantity)`. **Join** — A LEFT JOIN relationship between two models. Defined by a target model name and join key pairs (from the model's own foreign keys). Each model only stores direct joins — multi-hop paths like `customers.regions.name` are resolved at query time by walking each intermediate model's own joins. diff --git a/docs/database-support.md b/docs/database-support.md index 3414695e..e70256f6 100644 --- a/docs/database-support.md +++ b/docs/database-support.md @@ -38,7 +38,10 @@ Oracle. ## Aggregation support Most aggregations (`sum`, `avg`, `min`, `max`, `count`, `count_distinct`, -`first`, `last`, `weighted_avg`) work on every supported database. +`count_distinct_approx`, `first`, `last`, `weighted_avg`) work on every +supported database. `count_distinct_approx` is dialect-aware (see +[below](#count_distinct_approx-by-dialect)) but always available — it falls +back to an exact `COUNT(DISTINCT)` where there's no native function. `median`, `percentile`, the variance/stddev family (`stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`), and the paired statistics (`corr`, `covar_samp`, `covar_pop`) need dialect-specific handling @@ -55,6 +58,23 @@ because no standard syntax works everywhere: | SQL Server (T-SQL) | **no** | **no** | yes | yes (decomposed) | `MEDIAN` doesn't exist and T-SQL's `PERCENTILE_CONT` is window-only (no `WITHIN GROUP` aggregate form) — SLayer raises `NotImplementedError`. Native `STDEV`/`STDEVP`/`VAR`/`VARP` (slayer renames the canonical `STDDEV_*`/`VAR_*` names at emit time). `CORR`/`COVAR_*` use the same variance-decomposition formula as MySQL (`cov(x,y) = (var(x+y) − var(x) − var(y)) / 2`, `corr = cov / (stddev(x) · stddev(y))`). | | BigQuery | **no** | **no** | yes | yes | BigQuery has no `MEDIAN` aggregate, and its `PERCENTILE_CONT` is analytic-only (no `WITHIN GROUP` syntax) — the base class emit `PERCENTILE_CONT(p) WITHIN GROUP (ORDER BY x)` fails at runtime. If you need percentile on BigQuery, define a custom `Aggregation` using `APPROX_QUANTILES(x, 100)[OFFSET(N)]`. Native `STDDEV_SAMP`/`STDDEV_POP`/`VAR_SAMP`/`VAR_POP`/`CORR`/`COVAR_SAMP`/`COVAR_POP` (sqlglot may emit `VARIANCE` for `var_samp`). | +### `count_distinct_approx` by dialect + +`count_distinct_approx` emits each database's native approximate-distinct +function where one exists, and falls back to an **exact** `COUNT(DISTINCT)` +where it does not. The fallback is exact (more accurate, never approximate), +so results are always at least as precise as requested. The per-dialect +mapping lives in `SqlDialect.build_approx_count_distinct`. + +| Engine | Emitted SQL | +|---|---| +| DuckDB / Spark / Databricks | `approx_count_distinct(x)` | +| ClickHouse | `uniq(x)` | +| BigQuery / Snowflake / SQL Server (T-SQL) / Oracle | `APPROX_COUNT_DISTINCT(x)` | +| Trino / Presto | `approx_distinct(x)` | +| Redshift | `APPROXIMATE COUNT(DISTINCT x)` | +| Postgres / SQLite / MySQL | `COUNT(DISTINCT x)` (exact fallback) | + ### SQLite caveats SQLite has a much smaller built-in math/stat catalog than the other supported diff --git a/docs/dbt/dbt_import.md b/docs/dbt/dbt_import.md index 2fb4c0d2..d8aaaa23 100644 --- a/docs/dbt/dbt_import.md +++ b/docs/dbt/dbt_import.md @@ -136,17 +136,53 @@ Nothing to add — the underlying measure is already directly queryable. All three fold into a `ModelMeasure` on the source semantic model. Inputs are referenced by **bare ModelMeasure name**, so the formula parser resolves them locally: -- **Derived**: `formula: "metric_a + metric_b"` -- **Ratio**: `formula: "numerator / denominator"` -- **Cumulative (unbounded)**: `formula: "cumsum(measure_name)"` - -#### Unconverted metrics - -Some dbt metrics cannot be expressed as a `ModelMeasure`. They are reported in `ConversionResult.unconverted_metrics` and printed with an `UNCONVERTED` tag. Categories: - -- **Cumulative with window or grain_to_date**: SLayer's `cumsum` is unbounded. -- **Conversion metrics**: entity-based sequential event tracking is not supported. -- **Transform-name shadowing**: a dbt measure or metric named after a SLayer transform (`cumsum`, `lag`, `lead`, `change`, `change_pct`, `time_shift`, `rank`, `percent_rank`, `dense_rank`, `ntile`, `first`, `last`) is rejected — using it bare in a formula would shadow the transform. +- **Derived**: `formula: "metric_a + metric_b"`. An `offset_window` on a single-aggregate input (a measure or simple metric) lowers to a `time_shift`: a `1 month` offset becomes `time_shift(metric_a, -1, 'month')`. +- **Ratio**: `formula: "numerator / nullif(denominator, 0)"` — the denominator is NULL-guarded to prevent divide-by-zero. +- **Cumulative (unbounded)**: `formula: "cumsum(measure_name)"`. + +#### Supported mappings + +Every legal dbt construct that reaches the importer is either represented exactly or [failed cleanly](#clean-fail-unsupported). Represented exactly: + +| dbt construct | SLayer representation | +| --- | --- | +| Measure `agg: sum/avg/min/max/count/count_distinct/median` | `ModelMeasure` `col:` | +| Measure `agg: percentile` (continuous) | `col:percentile(p=)` | +| Measure `agg: count_distinct_approx` | `col:count_distinct_approx` (dialect-aware) | +| Measure `agg: sum_boolean` | `Column.sql = "CASE WHEN () THEN 1 ELSE 0 END"`, type `INT`, `col:sum` | +| Metric-level / per-input `filter` | pushed down into a leaf `Column.filter` (CASE-inside-aggregate) | +| Filter as string **or** list (`WhereFilterIntersection`) | AND-joined into one filter | +| Ratio metric | `num / nullif(den, 0)` | +| Derived metric | `ModelMeasure` formula over inputs | +| Derived input `offset_window` (single aggregate) | `time_shift(input, -, '')` (plural grains normalized) | +| Unbounded cumulative | `cumsum(measure)` | +| `config.meta`, semantic-model `label` | carried onto the corresponding entity's `meta` | + +#### Clean-fail / unsupported + +Constructs that cannot be expressed exactly are **failed cleanly** — never converted to approximate or wrong SQL. Each is routed to the conversion report with a category, severity, and documented workaround, and the raw construct is stashed into the owning entity's `meta` so nothing is silently lost. + +| dbt construct | Why | Workaround | +| --- | --- | --- | +| Cumulative `window` (rolling) | Query-grain-dependent re-aggregation | Use `cumsum(measure)` for an unbounded total | +| Cumulative `grain_to_date` | Reset-at-grain can't bake into a saved measure | `cumsum(measure)` + put the grain dimension in the query | +| Cumulative `period_agg` ≠ `first` | Only the default running total is exact | Use the default `period_agg` | +| Derived input `offset_to_grain` | No truncate-to-grain shift transform | Use `cumsum(...)` + grain dimension | +| `offset_window` on a ratio/derived input | Multi-aggregate offset isn't exactly expressible | Restructure as a multi-stage `source_queries` model | +| Non-standard granularity (e.g. `fortnight`) | Not a SLayer granularity | Use day/week/month/quarter/year | +| `non_additive_dimension` (semi-additive) | Not exactly expressible | `balance:last(