diff --git a/converters/README.md b/converters/README.md index 417d7663..ae260071 100644 --- a/converters/README.md +++ b/converters/README.md @@ -94,7 +94,7 @@ The top-level container. Maps to the root object in each vendor's format. | `ai_context` | Instructions, synonyms for AI tools | Map if vendor supports AI/LLM annotations | | `datasets` | Logical datasets (fact/dimension tables) | See dataset mapping below | | `relationships` | Foreign key connections | See relationship mapping below | -| `metrics` | Aggregate measures | See metric mapping below | +| `metrics` | Model-scoped aggregate measures | See metric mapping below | | `custom_extensions` | Vendor-specific metadata | Extract extensions matching the target vendor | ### Datasets @@ -108,6 +108,7 @@ Datasets represent logical tables (fact or dimension tables). They contain field | `primary_key` | Single or composite primary key | Map to vendor's PK syntax; note composite keys use arrays like `[order_id, line_number]` | | `unique_keys` | Alternative unique identifiers | Map if vendor supports unique constraints | | `fields` | Row-level attributes (columns) | See field mapping below | +| `metrics` | Dataset-scoped aggregate measures whose expressions resolve entirely within this dataset | See metric mapping below. A converter that reads only `semantic_model.metrics` will silently drop these | | `ai_context` | Synonyms and context for AI | Map if vendor supports semantic annotations | | `custom_extensions` | Vendor-specific metadata | Extract extensions matching the target vendor | @@ -166,7 +167,18 @@ means `from.product_id = to.id AND from.variant_id = to.variant_id`. The convert ### Metrics -Metrics are aggregate measures defined at the semantic model level. They can span multiple datasets via relationships. +Metrics are aggregate measures. They appear in two places, and **a converter must read both**: + +- `semantic_model.metrics`: model-scoped. May span multiple datasets via relationships. Expressions reference fields by qualified name (`SUM(orders.amount)`). +- `datasets[].metrics`: dataset-scoped. The expression may reference this dataset's declared fields, written `dataset.field` (`SUM(orders.net_amount)`), and the columns of its `source`, written unqualified (`SUM(tax)`). A qualified reference must name a declared field. + +Both placements use the identical metric structure, so the field mapping below applies to each. + +A converter that reads only `semantic_model.metrics` produces an incomplete model. That is a lossy conversion, not a valid one: it SHOULD warn, naming the metrics it dropped, and MUST NOT present the output as a faithful representation of the source. + +When the target format has only one metric namespace, hoist dataset-scoped metrics to the model level. This requires two changes: qualify the expression's unqualified column references with the dataset name, since references to declared fields are already qualified, and qualify the metric's own name as `dataset_name.metric_name`, since two datasets may each declare a metric with the same local name. If the target namespace disallows dots, use an equivalent encoding. + +See [Metric Scoping](../core-spec/spec.md#metric-scoping) for the full rules. | Ossie Field | Description | Converter Consideration | |-----------|-------------|------------------------| diff --git a/core-spec/ossie-schema.json b/core-spec/ossie-schema.json index edddac31..d5b70bf3 100644 --- a/core-spec/ossie-schema.json +++ b/core-spec/ossie-schema.json @@ -215,6 +215,13 @@ "$ref": "#/$defs/Field" } }, + "metrics": { + "type": "array", + "items": { + "$ref": "#/$defs/Metric" + }, + "description": "Metrics that aggregate data held by this dataset. The expression may reference this dataset's declared fields, written dataset_name.field_name, and the columns of its source, written unqualified; a column used only inside a metric does not need to be declared as a field. A qualified reference must name a declared field of this dataset. An expression referencing another dataset is model-scoped and belongs in semantic_model.metrics. This constrains the expression only, not the query: the metric is joined and grouped like any other, using the model's relationships. Names must be unique within the dataset and must not collide with a field name of this dataset. Referenced from outside the dataset as dataset_name.metric_name. See core-spec/spec.md#metric-scoping." + }, "custom_extensions": { "type": "array", "items": { @@ -336,7 +343,7 @@ "items": { "$ref": "#/$defs/Metric" }, - "description": "Quantifiable measures spanning datasets" + "description": "Model-scoped metrics. Expressions reference fields by qualified name (dataset.field). Names must be unique across the semantic model; reusing the name of a dataset-scoped metric should produce a warning rather than an error. An unqualified metric reference resolves to a model-scoped metric. See core-spec/spec.md#metric-scoping." }, "custom_extensions": { "type": "array", diff --git a/core-spec/spec.md b/core-spec/spec.md index 156cb1db..c26d60fd 100644 --- a/core-spec/spec.md +++ b/core-spec/spec.md @@ -37,7 +37,8 @@ 4. [Relationships](#relationships) 5. [Fields](#fields) 6. [Metrics](#metrics) -7. [Examples](#examples) +7. [Metric Scoping](#metric-scoping) +8. [Examples](#examples) --- @@ -92,7 +93,7 @@ The top-level container that represents a complete semantic model, including dat | `ai_context` | string/object | No | Additional context for AI tools (e.g., custom instructions) | | `datasets` | array | Yes | Collection of logical datasets (fact and dimension tables) | | `relationships` | array | No | Defines how logical datasets are connected | -| `metrics` | array | No | Quantifiable measures defined as aggregate expressions on fields from logical datasets | +| `metrics` | array | No | Model-scoped metrics: for expressions that must combine fields from more than one dataset. See [Metric Scoping](#metric-scoping). | | `custom_extensions` | array | No | Vendor-specific attributes for extensibility | ### Example @@ -130,6 +131,7 @@ Logical datasets represent business entities or concepts (fact and dimension tab | `description` | string | No | Human-readable description | | `ai_context` | string/object | No | Additional context for AI tools (e.g., synonyms, common terms) | | `fields` | array | No | Row-level attributes for grouping, filtering, and metric expressions | +| `metrics` | array | No | Dataset-scoped metrics whose expressions resolve entirely within this dataset. See [Metric Scoping](#metric-scoping). | | `custom_extensions` | array | No | Vendor-specific attributes | ### Primary Key Examples @@ -358,7 +360,14 @@ Common combinations: ## Metrics -Quantitative measures defined on business data, representing key calculations like sums, averages, ratios, etc. Metrics are defined at the semantic model level and can span multiple datasets. +Quantitative measures defined on business data, representing key calculations like sums, averages, ratios, etc. + +Metrics may be defined in two placements, using the same structure in both: + +- **Model-scoped** (`semantic_model.metrics`): for expressions that must combine fields from more than one dataset. +- **Dataset-scoped** (`datasets[].metrics`): aggregates data held by one dataset. Still joined and grouped through the model's relationships like any other metric. + +See [Metric Scoping](#metric-scoping) for the rules governing each. ### Schema @@ -417,6 +426,133 @@ expression: --- +## Metric Scoping + +A metric may be defined at the semantic model level or on an individual dataset. Both placements use the identical metric structure; only the resolution rules differ. + +| | Model-scoped (`semantic_model.metrics`) | Dataset-scoped (`datasets[].metrics`) | +|---|---|---| +| Expression references | Fields of any dataset in the model, qualified: `dataset.field` | Fields of its own dataset, qualified: `dataset.field`; columns of its `source`, unqualified: `column` | +| Name uniqueness | Unique across the semantic model | Unique within its dataset, and distinct from that dataset's field names | +| Referenced as | `metric_name` | `dataset_name.metric_name` | + +**Rules** + +1. A dataset-scoped metric's expression MAY reference the declared fields of its dataset and the columns of its `source`. A declared field is written `dataset_name.field_name`; a column of the `source` is written unqualified: `SUM(orders.net_amount) / COUNT(DISTINCT tax_id)`. A qualified reference MUST name a declared field of the declaring dataset. An expression that references another dataset MUST be declared in `semantic_model.metrics`. +2. Dataset-scoped metric names MUST be unique within their dataset. Two datasets MAY each declare a metric with the same name. +3. A dataset-scoped metric name MUST NOT collide with the name of a field of the same dataset, which under rule 5 would leave `orders.amount` ambiguous. +4. A model-scoped metric SHOULD NOT reuse the name of a dataset-scoped metric in the same semantic model. Validators SHOULD warn, and SHOULD attribute the warning to the model rather than to the dataset. +5. An unqualified metric reference resolves to a model-scoped metric. A dataset-scoped metric is referenced as `dataset_name.metric_name`. + +A column used only inside a metric does not need to be declared as a field. Declaring one is how a metric reuses a field's expression instead of repeating it. + +Because the two are spelled differently, a field and a source column MAY share a name without ambiguity. In a metric of `orders`, `orders.foo` is the declared field and `foo` is the source column, so declaring a field named after an existing column does not change the meaning of an expression already using the bare name. + +Whether a bare name is a real column of the `source` is not checked, because that requires catalog metadata the model does not carry. A qualified reference is checked, because the field list is in the model. + +Rule 1 places no limit on the complexity of the aggregation, and constrains the expression rather than the query. A dataset-scoped metric is joined and grouped like any other using the relationships declared in the model, so a metric declared on `store_sales` as `SUM(ss_ext_sales_price)` may still be grouped by `item.i_brand` or `store.s_state`. + +**Which names may repeat** + +Names are addressed in three ways, so a name may repeat across them without ambiguity: + +| Kind | Addressed as | +|---|---| +| Model-scoped metric | Bare: `revenue` | +| Dataset-scoped metric | Qualified: `orders.revenue` | +| Field | Qualified: `orders.revenue` | +| Dataset | Only ever as a qualifier | + +A model-scoped metric MAY therefore share a name with a field, or with a dataset. Rule 3 is an error because a field and a metric of one dataset share the same qualified namespace; rule 4 is a warning because the two names remain separately addressable. + +**Example: dataset-scoped metrics** + +```yaml +datasets: + - name: orders + source: sales.public.orders + primary_key: [order_id] + fields: + - name: order_id + expression: + dialects: + - dialect: ANSI_SQL + expression: order_id + description: Order identifier + - name: net_amount + expression: + dialects: + - dialect: ANSI_SQL + expression: amount - discount + description: Order amount after discount + + metrics: + # Qualified, so this references the declared field net_amount + - name: total_net_amount + expression: + dialects: + - dialect: ANSI_SQL + expression: SUM(orders.net_amount) + description: Total order amount after discount + datatype: Decimal + + # Unqualified, so this references tax, a column of the source that is + # not declared as a field + - name: total_tax + expression: + dialects: + - dialect: ANSI_SQL + expression: SUM(tax) + description: Total tax collected + datatype: Decimal + + - name: order_count + expression: + dialects: + - dialect: ANSI_SQL + expression: COUNT(order_id) + description: Number of orders + datatype: Integer +``` + +Referenced from a consumer as `orders.total_net_amount`, `orders.total_tax` and `orders.order_count`. + +**Example: invalid dataset-scoped metrics** + +```yaml +datasets: + - name: orders + source: sales.public.orders + fields: + - name: net_amount + expression: + dialects: + - dialect: ANSI_SQL + expression: amount - discount + metrics: + # INVALID (rule 1): references the customers dataset, so this metric is + # model-scoped and belongs in semantic_model.metrics + - name: revenue_per_customer + expression: + dialects: + - dialect: ANSI_SQL + expression: SUM(net_amount) / COUNT(DISTINCT customers.id) + + # INVALID (rule 1): a qualified reference must name a declared field, and + # tax is only a column of the source. Write it unqualified as SUM(tax) + - name: total_tax + expression: + dialects: + - dialect: ANSI_SQL + expression: SUM(orders.tax) +``` + +**Consumer guidance: flattening to a single metric namespace** + +Hoisting a dataset-scoped metric to the model level requires qualifying its field references with the dataset name, and qualifying the metric's own name as `dataset_name.metric_name` since two datasets may reuse a local name. Use an equivalent encoding if the target namespace disallows dots. + +A consumer that reads only `semantic_model.metrics` produces an incomplete representation of the model. This is a lossy conversion rather than a valid one: such a consumer SHOULD warn, naming the metrics it dropped, and MUST NOT present the result as a faithful representation of the source. Producers requiring maximum compatibility may continue declaring all metrics at the model level. + ## Custom Extensions Custom extensions allow vendors to add platform-specific metadata without breaking core compatibility. Each extension includes a vendor name and arbitrary JSON data. @@ -546,6 +682,16 @@ semantic_model: expression: amount description: Order amount + # Dataset-scoped: resolves entirely within the orders dataset + metrics: + - name: total_amount + expression: + dialects: + - dialect: ANSI_SQL + expression: SUM(amount) + description: Total order amount + datatype: Decimal + - name: customers source: sales.public.customers primary_key: [id] @@ -565,6 +711,20 @@ semantic_model: expression: email description: Customer email + # Dataset-scoped: resolves entirely within the customers dataset + metrics: + - name: customer_count + expression: + dialects: + - dialect: ANSI_SQL + expression: COUNT(DISTINCT id) + description: Total number of customers + datatype: Integer + ai_context: + synonyms: + - "total customers" + - "customer base" + relationships: - name: orders_to_customers from: orders @@ -573,27 +733,14 @@ semantic_model: to_columns: [id] metrics: - - name: total_revenue - expression: - dialects: - - dialect: ANSI_SQL - expression: SUM(orders.amount) - description: Total revenue from all orders - ai_context: - synonyms: - - "total sales" - - "revenue" - - - name: customer_count + # Model-scoped: spans orders and customers via the relationship + - name: revenue_per_customer expression: dialects: - dialect: ANSI_SQL - expression: COUNT(DISTINCT customers.id) - description: Total number of customers - ai_context: - synonyms: - - "total customers" - - "customer base" + expression: SUM(orders.amount) / COUNT(DISTINCT customers.id) + description: Average revenue per customer + datatype: Decimal custom_extensions: - vendor_name: SNOWFLAKE diff --git a/core-spec/spec.yaml b/core-spec/spec.yaml index 9b21b444..2e96028f 100644 --- a/core-spec/spec.yaml +++ b/core-spec/spec.yaml @@ -77,8 +77,16 @@ semantic_model: # See Relationships section below for detailed structure relationships: [] - # Optional: - # These metrics can span one or more logical datasets and use relationships + # Optional: Model-scoped metrics + # Expressions reference fields by qualified name (e.g., SUM(orders.amount)), + # unlike dataset-scoped metrics, which use unqualified names + # Names MUST be unique across the semantic model + # A model-scoped metric SHOULD NOT reuse the name of a dataset-scoped metric; + # validators warn rather than fail, and attribute the warning to the model + # An unqualified metric reference resolves to a model-scoped metric + # + # Whether a metric expression may reference another metric is not yet defined + # by this specification and is being handled as a separate proposal # See Metrics section below for detailed structure metrics: [] @@ -134,6 +142,29 @@ datasets: # See Fields section below for detailed structure fields: [] + # Optional: Dataset-scoped metrics + # Metrics that aggregate data held by this dataset. Same structure as + # model-scoped metrics; see the Metrics section below. + # + # Rules (see core-spec/spec.md#metric-scoping): + # - The expression MAY reference this dataset's declared fields and the + # columns of its source. A declared field is written + # dataset_name.field_name; a source column is written unqualified: + # SUM(orders.net_amount) / COUNT(DISTINCT tax_id). A column used only + # inside a metric does not need to be declared as a field. + # - A qualified reference MUST name a declared field of this dataset. + # - An expression referencing another dataset belongs in + # semantic_model.metrics. + # - Names MUST be unique within this dataset and MUST NOT collide with a + # field name of this dataset. + # - Referenced from outside the dataset as dataset_name.metric_name. An + # unqualified metric name refers to a model-scoped metric. + # + # These rules constrain the EXPRESSION only, not the query. The metric is + # joined and grouped like any other, using the model's relationships, so it + # can be sliced by dimensions of any dataset the model connects. + metrics: [] + # Optional: Vendor-specific attributes for extensibility custom_extensions: - vendor_name: string # Free-form string identifying the vendor @@ -235,6 +266,10 @@ fields: # Metrics Schema # Quantitative measures defined on business data # Represents key calculations like sums, averages, ratios, etc. +# +# The same structure is used in two placements: +# - semantic_model.metrics (model-scoped): may span datasets via relationships +# - datasets[].metrics (dataset-scoped): must resolve within one dataset metrics: # Required: Unique identifier for the metric - name: string diff --git a/docs/index.md b/docs/index.md index 3836092d..89617462 100644 --- a/docs/index.md +++ b/docs/index.md @@ -55,7 +55,7 @@ The Ossie core specification (current version: **0.2.0.dev0**, latest released: | **Datasets** | Logical datasets representing business entities (fact and dimension tables), with fields, primary keys, and unique keys. | | **Fields** | Row-level attributes for grouping, filtering, and metric expressions. Fields support multiple SQL dialects for cross-platform compatibility. | | **Relationships** | Foreign key connections between datasets, supporting both simple and composite keys. | -| **Metrics** | Quantitative measures (sums, averages, ratios, etc.) defined at the model level, capable of spanning multiple datasets. | +| **Metrics** | Quantitative measures (sums, averages, ratios, etc.). A model-scoped metric's expression may combine fields from several datasets; a dataset-scoped metric aggregates data held by one dataset. Either is joined and grouped through the model's relationships. | | **Custom Extensions** | Vendor-specific metadata stored as JSON, allowing platforms to carry additional information without breaking core compatibility. | | **AI Context** | Optional annotations at every level (model, dataset, field, relationship, metric) to help AI tools understand business meaning — including instructions, synonyms, and example queries. | @@ -248,7 +248,7 @@ YAML is more human-readable and easier to author by hand, which is important for Through `custom_extensions`. Each vendor can store arbitrary JSON metadata in extension blocks tagged with their vendor name. This metadata is preserved during round-trip conversions and ignored by tools that don't understand it — ensuring that no information is lost. **Can metrics reference multiple datasets?** -Yes. Metrics are defined at the semantic model level (not within a dataset) and can reference fields from multiple datasets. +Any metric can be *grouped and filtered* by fields from other datasets, using the relationships declared in the model. The placement affects only what the metric's own **expression** may reference. A model-scoped metric (`semantic_model.metrics`) may combine fields from several datasets in one expression, for example `SUM(orders.amount) / COUNT(DISTINCT customers.id)`. A dataset-scoped metric (`datasets[].metrics`) aggregates data held by a single dataset, and is still sliceable by any dataset the model connects. See [Metric Scoping](https://github.com/apache/ossie/blob/main/core-spec/spec.md#metric-scoping). **What SQL dialects are supported?** The current specification supports `ANSI_SQL`, `SNOWFLAKE`, `DATABRICKS`, `MDX`, and `TABLEAU`. New dialects can be proposed through the standard specification change process. @@ -318,7 +318,7 @@ A practical guide for organizations looking to adopt Ossie. | **Dataset** | A logical representation of a business entity, typically corresponding to a fact table or dimension table in a data warehouse. | | **Field** | A row-level attribute within a dataset, used for grouping, filtering, or as part of metric expressions. Fields can be simple column references or computed expressions. A field's logical data type is declared by the optional top-level `datatype` field (one of `String`, `Integer`, `Decimal`, `Float`, `Boolean`, `Date`, `Time`, `DateTime`, `DateTimeTz`, or `Opaque`). | | **Dimension** | A categorical attribute used to slice and filter data (e.g., region, product category, date). In Ossie, dimensions are represented as fields with optional metadata such as `is_time`. | -| **Metric** | A quantitative measure computed by aggregating data across one or more datasets (e.g., total revenue, average order value). Metrics are defined at the semantic model level. | +| **Metric** | A quantitative measure computed by aggregating data across one or more datasets (e.g., total revenue, average order value). Metrics may be defined at the semantic model level, or on an individual dataset when the expression resolves entirely within it. | | **Relationship** | A foreign key connection between two datasets, defining how they can be joined. Relationships are always many-to-one (from the referencing dataset to the referenced dataset). | | **Dialect** | A specific SQL or expression language variant (e.g., `ANSI_SQL`, `SNOWFLAKE`, `DATABRICKS`). Ossie supports multiple dialects so expressions can be tailored to each platform. | | **Custom Extension** | Vendor-specific metadata attached to any Ossie construct as a JSON string. Extensions allow platforms to carry additional information without modifying the core specification. | diff --git a/python/src/ossie/models.py b/python/src/ossie/models.py index 7ea515c3..a8423cd9 100644 --- a/python/src/ossie/models.py +++ b/python/src/ossie/models.py @@ -147,6 +147,24 @@ def is_time_dimension(self) -> bool: return self.datatype in _TEMPORAL_DATA_TYPES +class OssieMetric(BaseModel): + """Quantitative measure defined on business data. + + Used for both model-scoped metrics (OssieSemanticModel.metrics) and + dataset-scoped metrics (OssieDataset.metrics); the two placements share an + identical structure and differ only in their resolution rules. + """ + + model_config = ConfigDict(frozen=True) + + name: str + expression: OssieExpression + description: Optional[str] = None + datatype: Optional[OssieDataType] = None + ai_context: Optional[OssieAIContext] = None + custom_extensions: Optional[list[OssieCustomExtension]] = None + + class OssieDataset(BaseModel): """Logical dataset representing a business entity (fact or dimension table).""" @@ -159,6 +177,7 @@ class OssieDataset(BaseModel): description: Optional[str] = None ai_context: Optional[OssieAIContext] = None fields: Optional[list[OssieField]] = None + metrics: Optional[list[OssieMetric]] = None custom_extensions: Optional[list[OssieCustomExtension]] = None @@ -176,19 +195,6 @@ class OssieRelationship(BaseModel): custom_extensions: Optional[list[OssieCustomExtension]] = None -class OssieMetric(BaseModel): - """Quantitative measure defined on business data.""" - - model_config = ConfigDict(frozen=True) - - name: str - expression: OssieExpression - description: Optional[str] = None - datatype: Optional[OssieDataType] = None - ai_context: Optional[OssieAIContext] = None - custom_extensions: Optional[list[OssieCustomExtension]] = None - - class OssieSemanticModel(BaseModel): """Top-level container representing a complete semantic model.""" diff --git a/python/tests/test_models.py b/python/tests/test_models.py index 74d2a320..0c978aa2 100644 --- a/python/tests/test_models.py +++ b/python/tests/test_models.py @@ -135,3 +135,46 @@ def test_effective_time_dimension_role( ) assert field.is_time_dimension() is expected + + +def test_every_schema_property_exists_on_its_model() -> None: + """Guard against structural drift between the JSON Schema and these models. + + Pydantic's default ``extra='ignore'`` means a property present in the schema + but absent from the corresponding model is silently discarded on load, with + no validation error. That is how ``OssieDataset.metrics`` came to drop + dataset-scoped metrics. Comparing the enum lists alone does not catch it, so + walk every ``$defs`` entry that declares ``properties`` and assert each one + is representable. + """ + import ossie.models as models + + schema_path = Path(__file__).parents[2] / "core-spec" / "ossie-schema.json" + schema = json.loads(schema_path.read_text()) + + checked = 0 + for def_name, definition in schema["$defs"].items(): + properties = definition.get("properties") + if not properties: + continue + + model_name = f"Ossie{def_name}" + model = getattr(models, model_name, None) + assert model is not None, ( + f"schema defines $defs.{def_name} with properties but there is no " + f"{model_name} model to represent it" + ) + + # Compare against aliases too: `from` is a Python keyword, so + # OssieRelationship exposes it under an alias. + representable = { + field.alias or name for name, field in model.model_fields.items() + } + missing = sorted(set(properties) - representable) + assert not missing, ( + f"{model_name} cannot represent {missing} from $defs.{def_name}; " + f"these would be silently dropped on load" + ) + checked += 1 + + assert checked >= 9, f"expected to check at least 9 models, checked {checked}" diff --git a/validation/tests/test_validate.py b/validation/tests/test_validate.py index 30806f18..5d9fdb3a 100644 --- a/validation/tests/test_validate.py +++ b/validation/tests/test_validate.py @@ -161,3 +161,303 @@ def test_skips_malformed_flat_unique_keys() -> None: ) assert errors == [] + + +# --------------------------------------------------------------------------- +# Metric scoping and metric name uniqueness. +# +# Every test under "reported in review" corresponds to a defect found in review +# of apache/ossie#343 and fails against the validator as it stood before it. +# --------------------------------------------------------------------------- + +# The metric-scoping checks no-op without sqlglot, which would let the whole +# scoping section pass without asserting anything. Skip visibly instead. +pytest.importorskip("sqlglot") + +validate_metric_scoping = _VALIDATE.validate_metric_scoping +validate_unique_names = _VALIDATE.validate_unique_names +validate_sql = _VALIDATE.validate_sql + + +def _expr(sql: str, dialect: str = "ANSI_SQL") -> dict: + return {"dialects": [{"dialect": dialect, "expression": sql}]} + + +def _field(name: str) -> dict: + return {"name": name, "expression": _expr(name)} + + +def _metric(name: str, sql: str, dialect: str = "ANSI_SQL") -> dict: + return {"name": name, "expression": _expr(sql, dialect)} + + +def _metric_document( + dataset_metrics: list[dict] | None = None, + fields: list[dict] | None = None, + extra_datasets: list[dict] | None = None, + model_metrics: list[dict] | None = None, +) -> dict: + orders = { + "name": "orders", + "source": "db.s.orders", + "fields": fields if fields is not None else [_field("amount")], + "metrics": dataset_metrics or [], + } + return { + "version": "0.2.0.dev0", + "semantic_model": [ + { + "name": "m", + "datasets": [orders] + (extra_datasets or []), + "metrics": model_metrics or [], + } + ], + } + + +_CUSTOMERS_WITH_ID = { + "name": "customers", + "source": "db.s.customers", + "fields": [_field("id")], +} + + +# --- scoping rules --------------------------------------------------------- + + +def test_dataset_scoped_metric_accepts_unqualified_column_reference() -> None: + doc = _metric_document([_metric("total", "SUM(amount)")]) + + assert validate_metric_scoping(doc) == [] + + +def test_dataset_scoped_metric_accepts_a_qualified_declared_field() -> None: + # A qualifier names a declared field, which is how a metric reuses a + # field's expression rather than repeating it. `amount` is declared here. + doc = _metric_document([_metric("total", "SUM(orders.amount)")]) + + assert validate_metric_scoping(doc) == [] + + +def test_dataset_scoped_metric_rejects_a_qualified_undeclared_name() -> None: + # `tax` is a column of the source, not a declared field, so the qualified + # spelling is wrong: it should be written SUM(tax). + errors = validate_metric_scoping( + _metric_document([_metric("total", "SUM(orders.tax)")]) + ) + + assert any("MUST name a declared field" in error for error in errors) + + +def test_dataset_scoped_metric_rejects_another_dataset() -> None: + doc = _metric_document( + [_metric("bad", "SUM(amount) / COUNT(DISTINCT customers.id)")], + extra_datasets=[_CUSTOMERS_WITH_ID], + ) + errors = validate_metric_scoping(doc) + + assert any("references dataset(s) 'customers'" in error for error in errors) + + +def test_dataset_scope_does_not_limit_aggregation_complexity() -> None: + # Rule 1 limits what an expression may reach, not its complexity. + doc = _metric_document( + [_metric("odd", "SUM(CASE WHEN amount > 0 THEN amount ELSE 0 END) / NULLIF(COUNT(*), 0)")] + ) + + assert validate_metric_scoping(doc) == [] + + +# --- reported in review ---------------------------------------------------- + + +def test_uppercase_qualifier_is_not_reported_as_a_missing_dataset() -> None: + # SQL identifiers are case-insensitive, but the qualifier was compared + # against the raw YAML name, so SUM(ORDERS.AMOUNT) on dataset `orders` was + # rejected while naming a dataset that does not exist. Both the qualifier + # and the field name it qualifies must be matched case-insensitively. + errors = validate_metric_scoping( + _metric_document([_metric("total", "SUM(ORDERS.AMOUNT)", "SNOWFLAKE")]) + ) + + assert errors == [] + + +def test_struct_path_is_not_reported_as_a_dataset() -> None: + # sqlglot places the middle element of a three-part path in Column.table, + # so reading `table` alone reported the struct field as a dataset. + doc = _metric_document( + [_metric("total", "SUM(payload.amount)")], fields=[_field("payload")] + ) + + assert validate_metric_scoping(doc) == [] + + +def test_three_part_path_reports_the_dataset_not_the_struct_field() -> None: + # sqlglot places the middle element of a three-part path in Column.table, + # so reading `table` alone reported the struct field as a dataset. The + # qualifier must read as `orders` and the name it qualifies as `payload`, + # which is undeclared here. + errors = validate_metric_scoping( + _metric_document([_metric("total", "SUM(orders.payload.amount)")]) + ) + + assert any("'orders.payload'" in error for error in errors) + assert not any("references dataset(s)" in error for error in errors) + + +def test_three_part_path_to_a_declared_struct_field_is_valid() -> None: + doc = _metric_document( + [_metric("total", "SUM(orders.payload.amount)")], fields=[_field("payload")] + ) + + assert validate_metric_scoping(doc) == [] + + +def test_local_alias_is_not_reported_as_a_dataset() -> None: + doc = _metric_document([_metric("total", "SUM(o.amount)")]) + + assert validate_metric_scoping(doc) == [] + + +def test_subquery_source_is_not_reported_as_a_dataset() -> None: + doc = _metric_document( + [_metric("total", "(SELECT SUM(x) FROM other WHERE other.id = amount)")] + ) + + assert validate_metric_scoping(doc) == [] + + +def test_collision_report_order_is_deterministic() -> None: + # Set iteration order varies between processes under hash randomisation. + names = ("a", "b", "c", "d") + doc = _metric_document( + [_metric(name, "SUM(amount)") for name in names], + model_metrics=[_metric(name, "SUM(orders.amount)") for name in names], + ) + + assert len({tuple(validate_unique_names(doc)) for _ in range(8)}) == 1 + + +def test_null_expression_does_not_raise() -> None: + # `.get(key, {})` returns None when the key is present but null, so a + # truncated hand-edit produced a traceback instead of the schema error. + doc = { + "version": "0.2.0.dev0", + "semantic_model": [ + { + "name": "m", + "datasets": [ + { + "name": "orders", + "source": "db.s.orders", + "metrics": [{"name": "total", "expression": None}], + } + ], + "metrics": [{"name": "other", "expression": None}], + } + ], + } + + validate_sql(doc) + validate_metric_scoping(doc) + + +def test_expression_is_parsed_once_across_checks() -> None: + _VALIDATE._parse_expression.cache_clear() + doc = _metric_document([_metric("total", "SUM(amount)")]) + before = _VALIDATE._parse_expression.cache_info().hits + + validate_metric_scoping(doc) + validate_sql(doc) + + assert _VALIDATE._parse_expression.cache_info().hits > before + + +# --- metric name rules ----------------------------------------------------- + + +def test_metric_may_not_take_the_name_of_a_field_of_its_dataset() -> None: + # Both occupy `orders.amount`, so the reference would resolve two ways. + errors = validate_unique_names(_metric_document([_metric("amount", "SUM(amount)")])) + + assert any("collides with field" in error for error in errors) + assert not any("Warning:" in error for error in errors) + + +def test_duplicate_metric_name_within_a_dataset_is_an_error() -> None: + doc = _metric_document( + [_metric("total", "SUM(amount)"), _metric("total", "COUNT(*)")] + ) + errors = validate_unique_names(doc) + + assert any("Duplicate metric name 'total'" in error for error in errors) + + +def test_two_datasets_may_reuse_a_metric_name() -> None: + # Dataset-scoped names are scoped to their dataset. + shipments = { + "name": "shipments", + "source": "db.s.shipments", + "fields": [_field("qty")], + "metrics": [_metric("item_count", "COUNT(*)")], + } + doc = _metric_document( + [_metric("item_count", "COUNT(*)")], extra_datasets=[shipments] + ) + + assert validate_unique_names(doc) == [] + + +def test_model_metric_shadowing_a_dataset_metric_only_warns() -> None: + # A dataset may be authored independently and reused across models, so it + # must not become invalid because of a name the surrounding model adds. + doc = _metric_document( + [_metric("total_sales", "SUM(amount)")], + model_metrics=[_metric("total_sales", "SUM(orders.amount)")], + ) + errors = validate_unique_names(doc) + + assert errors == [ + "[Unique] Warning: model-scoped metric 'total_sales' in model 'm' shadows " + "dataset-scoped metric 'orders.total_sales'. Both remain addressable, but " + "consumers resolving the name 'total_sales' cannot tell which was " + "intended; consider renaming the model-scoped metric." + ] + + +@pytest.mark.parametrize("name", ["amount", "orders"]) +def test_model_metric_may_reuse_a_field_or_dataset_name(name: str) -> None: + # Bare and qualified names never collide, and a model-scoped metric usually + # takes the name of the column it aggregates, so this is left permitted. + doc = _metric_document(model_metrics=[_metric(name, "SUM(orders.amount)")]) + + assert validate_unique_names(doc) == [] + + +def test_metric_may_reference_an_undeclared_source_column() -> None: + # A dataset-scoped metric's expression is written against the dataset's + # source, so a column used only inside a metric need not be declared as a + # field first. Here 'tax' is not in fields. + doc = _metric_document( + [_metric("total_tax", "SUM(tax)")], + fields=[_field("amount")], + ) + + assert validate_metric_scoping(doc) == [] + + +def test_declared_field_and_source_column_may_share_a_name() -> None: + # A field `foo` whose expression is not simply `foo` does not shadow the + # source column `foo`. The two spellings keep them separately reachable: + # `orders.foo` is the field, bare `foo` is the column. + shadowing_field = {"name": "foo", "expression": _expr("UPPER(bar)")} + doc = _metric_document( + [ + _metric("via_field", "COUNT(DISTINCT orders.foo)"), + _metric("via_column", "COUNT(DISTINCT foo)"), + ], + fields=[shadowing_field], + ) + + assert validate_metric_scoping(doc) == [] diff --git a/validation/validate.py b/validation/validate.py index 8757bdf1..4e3a9da8 100644 --- a/validation/validate.py +++ b/validation/validate.py @@ -33,7 +33,9 @@ 1. JSON Schema (structure, types, enums) 2. Unique names (datasets, fields, metrics, relationships) 3. Valid relationship references -4. SQL syntax (using sqlglot) +4. Metric scoping (a dataset-scoped metric's expression references its dataset's + declared fields as dataset.field, and its source's columns unqualified) +5. SQL syntax (using sqlglot) Usage: python validation/validate.py @@ -43,6 +45,7 @@ import json import sys +from functools import lru_cache from pathlib import Path try: @@ -55,6 +58,7 @@ try: import sqlglot + from sqlglot import exp from sqlglot.errors import ParseError, TokenError SQLGLOT_AVAILABLE = True except ImportError: @@ -120,6 +124,43 @@ def validate_unique_names(data: dict) -> list[str]: for dup in find_duplicates(metric_names): errors.append(f"[Unique] Duplicate metric name '{dup}' in model '{model_name}'") + # Check unique dataset-scoped metric names within each dataset, and that + # they do not shadow a field of the same dataset. A model-scoped metric + # reusing the name is reported separately, as a warning against the model. + model_metric_names = set(metric_names) + for dataset in model.get("datasets", []): + dataset_name = dataset.get("name", "") + ds_metric_names = [ + m.get("name") for m in dataset.get("metrics", []) if m.get("name") + ] + ds_field_names = { + f.get("name") for f in dataset.get("fields", []) if f.get("name") + } + for dup in find_duplicates(ds_metric_names): + errors.append( + f"[Unique] Duplicate metric name '{dup}' in dataset '{dataset_name}'" + ) + for name in sorted(set(ds_metric_names) & ds_field_names): + errors.append( + f"[Unique] Dataset-scoped metric '{dataset_name}.{name}' collides with " + f"field '{dataset_name}.{name}'; '{dataset_name}.{name}' would be " + f"ambiguous between a row-level field and an aggregate" + ) + for name in sorted(set(ds_metric_names) & model_metric_names): + # Reported as a warning, and attributed to the model rather than + # the dataset. A dataset may be authored independently and reused + # across models, so it must not become invalid because of a name + # the surrounding model happens to introduce. References stay + # unambiguous either way: '' is the model-scoped metric and + # '.' is the dataset-scoped one. + errors.append( + f"[Unique] Warning: model-scoped metric '{name}' in model " + f"'{model_name}' shadows dataset-scoped metric " + f"'{dataset_name}.{name}'. Both remain addressable, but " + f"consumers resolving the name '{name}' cannot tell which was " + f"intended; consider renaming the model-scoped metric." + ) + # Check unique relationship names rel_names = [r.get("name") for r in model.get("relationships", []) if r.get("name")] for dup in find_duplicates(rel_names): @@ -167,29 +208,158 @@ def validate_references(data: dict) -> list[str]: return errors -def validate_sql_expression(expr: str, dialect: str, context: str) -> str | None: - """Validate a single SQL expression. Returns error message or None if valid.""" - if not SQLGLOT_AVAILABLE: - return None +@lru_cache(maxsize=2048) +def _parse_expression(expr: str, dialect: str): + """Parse an expression, trying it bare and then wrapped in SELECT. + + Returns ``(tree, error)``. Exactly one of the two is meaningful: - if dialect in SKIP_SQL_VALIDATION: - return None + * ``(tree, None)`` - parsed successfully. + * ``(None, message)`` - sqlglot rejected the expression in both forms. + * ``(None, None)`` - parsing was skipped, either because sqlglot is not + installed or because the dialect is one sqlglot cannot parse. Callers + must treat this as "unknown" rather than as a failure. + + Results are cached because several checks run over the same expressions. + The returned tree is shared between callers and MUST NOT be mutated. + """ + if not SQLGLOT_AVAILABLE or dialect in SKIP_SQL_VALIDATION: + return None, None sqlglot_dialect = DIALECT_MAP.get(dialect) + error = None - try: - # Try parsing as expression first (for field expressions like "column_name") - sqlglot.parse_one(expr, dialect=sqlglot_dialect) - return None - except (ParseError, TokenError): - pass + for candidate in (expr, f"SELECT {expr}"): + try: + tree = sqlglot.parse_one(candidate, dialect=sqlglot_dialect) + except (ParseError, TokenError) as exc: + if error is None: + error = str(exc).split(chr(10))[0] + continue + if tree is not None: + return tree, None + + return None, error + + +def _qualified_references(tree) -> set[tuple[str, str]]: + """Every qualified column path in an expression, as (qualifier, name). + + ``orders.amount`` yields ``{("orders", "amount")}``. For a three-part path + such as ``payload.attrs.value``, sqlglot puts the middle part in + ``Column.table`` and the first in ``Column.db``, so reading ``table`` alone + would report ``attrs``. Taking ``parts[0]`` and ``parts[1]`` gives the + outermost qualifier and the name it qualifies in every case. Unqualified + columns contribute nothing. + """ + refs = set() + for col in tree.find_all(exp.Column): + parts = [part.name for part in col.parts] + if len(parts) > 1 and parts[0] and parts[1]: + refs.add((parts[0], parts[1])) + return refs + + +def validate_metric_scoping(data: dict) -> list[str]: + """Validate the expression rules for dataset-scoped metrics. + + A dataset-scoped metric's expression may reference the declared fields of + its dataset, written dataset_name.field_name, and the columns of its + source, written unqualified. Two things are errors: a qualifier naming + another dataset, since such a metric belongs in semantic_model.metrics, and + a qualifier naming the declaring dataset followed by a name that is not one + of its declared fields. + + Whether a bare name is a real column of the source is not checked, because + that needs catalog metadata the model does not carry. A qualified reference + is checked, because the field list is in the model. + + This checks the expression only. It says nothing about how the metric may be + queried: a dataset-scoped metric is joined and grouped like any other, using + the model's relationships. + + A qualifier that names neither the declaring dataset nor another dataset is + left alone: it is a local alias, CTE, or subquery source. + """ + errors = [] + + for model in data.get("semantic_model", []): + model_name = model.get("name", "") - try: - # Try wrapping in SELECT for simple column references - sqlglot.parse_one(f"SELECT {expr}", dialect=sqlglot_dialect) - return None - except (ParseError, TokenError) as e: - return f"[SQL] {context}: {str(e).split(chr(10))[0]}" + datasets = model.get("datasets", []) + all_dataset_names = { + d["name"].casefold() for d in datasets if d.get("name") + } + + for dataset in datasets: + dataset_name = dataset.get("name", "") + own_name = dataset_name.casefold() + other_dataset_names = all_dataset_names - {own_name} + own_field_names = { + f["name"].casefold() + for f in dataset.get("fields", []) + if f.get("name") + } + + for metric in dataset.get("metrics", []): + metric_name = metric.get("name", "") + expression = metric.get("expression") or {} + + for dialect_expr in expression.get("dialects", []): + dialect = dialect_expr.get("dialect", "ANSI_SQL") + expr = dialect_expr.get("expression", "") + if not expr: + continue + + tree, _ = _parse_expression(expr, dialect) + if tree is None: + continue + + foreign = set() + undeclared = set() + + for qualifier, referenced in _qualified_references(tree): + folded = qualifier.casefold() + if folded == own_name: + # Qualifying with the declaring dataset's own name + # is a reference to one of its declared fields. + if referenced.casefold() not in own_field_names: + undeclared.add(f"{qualifier}.{referenced}") + elif folded in other_dataset_names: + foreign.add(qualifier) + # Anything else is a struct or variant path, or a local + # alias, so it is not a dataset reference and needs no + # report. + + if foreign: + errors.append( + f"[Scope] Dataset-scoped metric '{dataset_name}.{metric_name}' " + f"in model '{model_name}' ({dialect}) references " + f"dataset(s) {', '.join(repr(f) for f in sorted(foreign))}. " + f"Dataset-scoped metrics aggregate one dataset; a " + f"metric spanning datasets is model-scoped and " + f"belongs in semantic_model.metrics." + ) + + if undeclared: + errors.append( + f"[Scope] Dataset-scoped metric '{dataset_name}.{metric_name}' " + f"in model '{model_name}' ({dialect}) references " + f"{', '.join(repr(u) for u in sorted(undeclared))}. " + f"A qualified reference MUST name a declared field " + f"of dataset '{dataset_name}'; reference a column of " + f"its source by unqualified name instead." + ) + + return errors + + +def validate_sql_expression(expr: str, dialect: str, context: str) -> str | None: + """Validate a single SQL expression. Returns error message or None if valid.""" + _, error = _parse_expression(expr, dialect) + if error: + return f"[SQL] {context}: {error}" + return None def validate_sql(data: dict) -> list[str]: @@ -211,7 +381,7 @@ def validate_sql(data: dict) -> list[str]: dataset_name = dataset.get("name", "") for field in dataset.get("fields", []): field_name = field.get("name", "") - expression = field.get("expression", {}) + expression = field.get("expression") or {} for dialect_expr in expression.get("dialects", []): dialect = dialect_expr.get("dialect", "ANSI_SQL") expr = dialect_expr.get("expression", "") @@ -221,10 +391,23 @@ def validate_sql(data: dict) -> list[str]: if error: errors.append(error) + # Validate dataset-scoped metric expressions + for metric in dataset.get("metrics", []): + metric_name = metric.get("name", "") + expression = metric.get("expression") or {} + for dialect_expr in expression.get("dialects", []): + dialect = dialect_expr.get("dialect", "ANSI_SQL") + expr = dialect_expr.get("expression", "") + if expr: + context = f"Metric '{dataset_name}.{metric_name}' in model '{model_name}' ({dialect})" + error = validate_sql_expression(expr, dialect, context) + if error: + errors.append(error) + # Validate metric expressions for metric in model.get("metrics", []): metric_name = metric.get("name", "") - expression = metric.get("expression", {}) + expression = metric.get("expression") or {} for dialect_expr in expression.get("dialects", []): dialect = dialect_expr.get("dialect", "ANSI_SQL") expr = dialect_expr.get("expression", "") @@ -280,6 +463,7 @@ def main(): if data.get("semantic_model"): errors.extend(validate_unique_names(data)) errors.extend(validate_references(data)) + errors.extend(validate_metric_scoping(data)) errors.extend(validate_sql(data)) # Report results