Skip to content

validation: check name-resolution rules from #486 #602

Open

Description

@justint-db

#486 defines how names resolve in a model (spec.md, "Name Resolution"). validation/validate.py doesn't check this yet.

validate.py doesn't connect to a warehouse, so it can't see source columns, but it can still check the following.

1. Key and relationship names are declared fields.
Every name in primary_key, unique_keys, from_columns, and to_columns must be a declared field of their corresponding dataset.

2. Metric references are qualified and resolve.
Every reference in a metric expression must be written dataset.field and resolve to a declared field. This applies to dialects sqlglot can parse.

3. Shadowing warning.
Warn when a field expression uses the name of another field in the same dataset, unless that field is a pass-through of the same-named column. Under #486 that name means the source column, which may not be what the author intended.

Checks 1 and 2 will start failing some existing documents, so they should be rolled out deliberately (errors vs. warnings, and a note in the changelog). Check 3 is a warning only.

Related: #486, #343 (dataset-scoped metrics may reference source columns unqualified, so check 2 needs to account for them when that lands).

Activity

  1. kayemkim commented on Oct 18, 2026

    @kayemkim
    Contributor

    I have checks 1 and 2 on a branch that has been waiting for #486 to land, since its messages cite the Name Resolution section that PR adds. Keys and relationship columns are looked up in the dataset's declared fields, metric expressions are parsed with sqlglot and every column has to be dataset.field naming a declared field, with the dialects sqlglot cannot parse skipped and a parse failure left to the SQL check. Both are warnings to start, for the reason you give, with the promotion to errors as a follow-up once the documents in the tree are clean. It comes with a small example whose field names all differ from their source columns, which validation-ci validates.

    Check 3 is not in it yet. I will add the shadowing warning to the same branch and open the PR against this issue once #486 is merged, unless you would rather carry it in a follow-up of your own, in which case I will send the branch over.

  2. kayemkim commented on Oct 19, 2026

    @kayemkim
    Contributor

    Opened #608 with the three checks, as warnings. The messages follow the Name Resolution text as it stands in #486 and will track it if that wording changes before it lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIssue needs triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions