Skip to content

fix: preserve metric semantics across fact sources - #6029

Open
tchivs wants to merge 2 commits into
SQLMesh:mainfrom
tchivs:feat/metrics-production
Open

fix: preserve metric semantics across fact sources#6029
tchivs wants to merge 2 commits into
SQLMesh:mainfrom
tchivs:feat/metrics-production

Conversation

@tchivs

@tchivs tchivs commented Sep 5, 2026

Copy link
Copy Markdown

Description

Fix correctness gaps in the existing Metrics prototype without introducing a separate semantic layer or serving API.

Dependency resolution

  • Resolve metric dependencies iteratively, preserving forward references and shared dependencies.
  • Report cycles and unknown references as ConfigError, including the dependency chain and defining file.
  • Resolve metric names case-insensitively, including quoted references.

Query correctness

  • Resolve and apply the complete WHERE predicate independently for each fact before aggregation. Previously, a shared filter affected only the first fact, so a filtered ratio could use an unfiltered denominator and grouped queries could return out-of-scope aggregates.
  • Preserve right-only and later-source group keys across FULL JOINs, including three facts and nullable composite keys.
  • Preserve computed grouping expressions, explicit dimension aliases, nested query scopes, DISTINCT ON, and named WINDOW references.
  • Correct terminal reference selection without bypassing many-to-many safeguards.
  • On PostgreSQL, use composite-key equality for nullable FULL JOIN keys: PostgreSQL cannot plan these joins with IS NOT DISTINCT FROM conditions.

Replace implementation-level SQL snapshots/commented scenarios with executable result/error regressions, add native PostgreSQL model-to-query integration coverage, and document the repeatable acceptance procedure.

Scope and limitations

  • Metrics remains explicitly marked as a prototype, not production-ready.
  • Unsupported metric WHERE subqueries, grouping sets, unresolved/ambiguous dimensions, and unsafe reference paths fail explicitly rather than silently weakening query semantics. Changing-key multi-hop reference paths are not implemented here.
  • PostgreSQL group dimensions must have matching types, or be normalized with explicit casts. Metric arithmetic follows the definition dialect; fractional integer ratios and zero-denominator behavior should be expressed explicitly.
  • Application authorization, a serving layer, Doris support, and additional metric types are outside this PR.

Test Plan

  • make style: passed (ruff, ruff-format, mypy, migration validation).
  • make fast-test: passed after installing the missing local unixODBC runtime dependency:
    • fast phase: 2664 passed, 4 skipped;
    • isolated phase: 3 passed;
    • registry-isolation phase: 1 passed;
    • dialect-isolation phase: 161 passed.
  • Focused Metrics, ReferenceGraph, and native PostgreSQL suite: 54 passed.
  • PostgreSQL 16.14, in a disposable isolated container: SQLMesh loads a temporary project, applies its models, compiles metric queries using Context.rewrite, and executes the generated SQL on PostgreSQL. In-memory DuckDB is used only for isolated SQLMesh state, following the existing integration fixtures.
  • Native coverage includes shared filters, decimal/count ratios, empty scopes, three-fact nullable composite groups, dimension filters, conditional aggregation, leap-day/time-boundary bucketing, quoted identifiers, and DISTINCT ON.
  • Native Context plan/rewrite/execute smoke verified returned results and cleanup. No business databases were used.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

Resolve metric dependencies iteratively and report cycles or unknown references with their dependency chain and defining file. Preserve forward references, shared dependencies, and case-insensitive metric names with execution-based regression coverage.

Signed-off-by: tchivs <topivn@live.cn>
Resolve complete predicates for every contributing fact before aggregation. Preserve full-join grouping keys, including NULL and composite keys, and retain outer SQL scope references. Use PostgreSQL-compatible composite equality for nullable grouping keys and add native model-to-query acceptance coverage. Reject unsupported query shapes explicitly while retaining the Metrics prototype status.

Signed-off-by: tchivs <topivn@live.cn>
@tchivs
tchivs force-pushed the feat/metrics-production branch from c5f78f0 to d8e1b00 Compare September 5, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant