Skip to content

Fix/required dimensions role support - #2561

Draft
shangyian wants to merge 5 commits into
DataJunction:mainfrom
shangyian:fix/required-dimensions-role-support
Draft

shangyian wants to merge 5 commits into
DataJunction:mainfrom
shangyian:fix/required-dimensions-role-support

Conversation

@shangyian

Copy link
Copy Markdown
Collaborator

Summary

Test Plan

  • PR has an associated issue: #
  • make check passes
  • make test shows 100% unit test coverage

Deployment Plan

A metric's required_dimensions could reference a role-qualified
dimension attribute (e.g. date.dateint[created_date]) but the role was
silently stripped during validation. This caused three problems:
non-deterministic query-time join resolution, no validation that a
declared role actually resolves to a reachable dimension link, and
deploy round-trips that never converge because export could never
reproduce a role that was already dropped at write time.

Replace the metric_required_dimensions join table (which stored an FK
to a Column, shared across roles) with a normalized shape that stores
the authored reference verbatim (`ref`) plus an indexed FK to the
dimension node (`dimension_id`, nullable for a bare parent-column
ref). This makes required_dimensions round-trip exactly through
export/diff, lets query-time resolution use the exact declared role
instead of a fallback sort, and lets dimension-deletion impact
analysis see metrics that depend on a dimension only through
required_dimensions.

Includes a migration with a backfill for existing rows, updated
validation (role reachability against a direct parent's dimension
links, including through derived-metric composition), and updated
read/write/build paths.
@netlify

netlify Bot commented Sep 22, 2026

Copy link
Copy Markdown

Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit b616553
🔍 Latest deploy log https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/6ab2a43c3fab7b0008c2ca7c

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