Repository navigation
feat(core-spec): support dataset-scoped metrics - #591
Conversation
Allow metrics to be declared on an individual dataset (datasets[].metrics) in addition to the semantic model (semantic_model.metrics), using the same metric structure in both placements. Dataset-scoped metrics are for aggregations that resolve entirely within a single dataset. They keep a metric next to the fields it depends on and make a dataset independently interpretable. Metrics that span datasets via relationships remain model-scoped. Scoping rules: - A dataset-scoped metric expression must only reference fields of its own dataset; it must not traverse relationships. - Names must be unique within their dataset. Two datasets may each declare a metric with the same local name. - A dataset-scoped name must not collide with any model-scoped metric name, keeping unqualified metric references unambiguous. - Referenced from outside the dataset as dataset_name.metric_name, mirroring how a dataset's fields are already referenced in metric expressions. Prior art: dbt MetricFlow (v1.12+) supports the same two-placement split, reserving in-model metrics for single-semantic-model metrics and top-level metrics for cross-model ones. Cube scopes measures to cubes with qualified cube_name.member references. Ossie follows Cube's scoped-uniqueness and qualified-reference convention because it matches how Ossie already treats fields. Dataset-scoped metrics reuse the existing Metric schema definition, so they inherit any future additions to the metric shape automatically. validate.py gains three checks that JSON Schema cannot express: duplicate metric names within a dataset, collisions with model-scoped metric names, and cross-dataset references in dataset-scoped expressions. Scope checking degrades gracefully for dialects sqlglot cannot parse (MDX, TABLEAU, MAQL) rather than reporting false positives. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Add Snowflake semantic views and Databricks Unity Catalog metric views to the prior-art comparison, alongside dbt MetricFlow and Cube. Snowflake is the closest analogue: table-level metrics scoped to a logical table plus top-level derived metrics that combine metrics across tables, with qualified table.metric references. Databricks takes a different approach, with a single flat scope per metric view and joins declared inside the view. Also records that this proposal is deliberately stricter than Snowflake on scope enforcement: Snowflake permits table-level metrics to traverse relationships via using_relationships, whereas dataset-scoped metrics here must resolve within their own dataset. Notes the rationale (a strict boundary is legible and can be relaxed compatibly later) and leaves the question open for the community. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Add AtScale SML as a fifth reference point. SML demonstrates a third placement pattern: a standalone, globally-named metric object that declares its binding by property (dataset and column are both required), with cross-entity calculations as a separate metric_calc object type. This positions the proposal between the extremes rather than at one end: SML binds a plain metric to a single column with a single aggregation method; Snowflake permits table-level metrics to traverse relationships; Ossie permits an arbitrary expression over the declaring dataset but no traversal. Also adds consumer guidance on flattening to a single metric namespace. Because a dataset-scoped expression resolves within its declaring dataset, it is already valid as a model-scoped metric, so consumers that support only model-level metrics can hoist without rewriting expressions. Notes the two real caveats: flattening requires name qualification, and consumers reading only semantic_model.metrics will not observe dataset-scoped metrics. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Move the three single-dataset metrics in the TPC-DS example into store_sales.metrics, leaving the two that genuinely span datasets at model level. The example now demonstrates the placement decision rather than contradicting the guidance in spec.md. Dataset-scoped (expressions resolve within store_sales): - total_sales, total_profit, sales_by_brand Model-scoped (span datasets via relationships): - customer_lifetime_value (store_sales + customer) - store_productivity (store_sales + store) Referencing these three changes from total_sales to store_sales.total_sales. Also clarifies in spec.md and spec.yaml that the scoping rule constrains a metric's expression, not how it may be queried. A dataset-scoped metric can still be grouped by or filtered on dimensions of other datasets reached through relationships, since grouping dimensions are supplied by the consumer at query time. The existing sales_by_brand metric is exactly this case: its expression touches only store_sales, but its description notes it "requires grouping by item.i_brand". Without this clarification the rule is easy to misread as forbidding cross-dataset grouping, which would make the feature appear far more limited than it is. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Incorporates review from jbonofre, christianeu-db and khush-bhatia on apache#343. Two substantive changes: - Dataset-scoped metric expressions now reference fields by unqualified name (SUM(amount), not SUM(orders.amount)), matching how a field's own expression is written. Raised by christianeu-db. - Name-collision rules are split into errors and warnings. A model-scoped metric reusing a dataset-scoped metric's name now warns instead of failing, and the warning is attributed to the model rather than the dataset, since a dataset may be authored independently and reused across models. Also reframes the placement throughout: a dataset-scoped metric aggregates data held by its dataset, and is still joined and grouped through the model's relationships like any other metric. The earlier "no traversal" wording implied a restriction on how the metric could be queried, which was wrong. Validator fixes, each reported with a reproduction: - Case-fold qualifier comparison, so SUM(ORDERS.AMOUNT) on dataset orders is no longer rejected against a dataset that does not exist - Use Column.parts[0] rather than Column.table, so STRUCT and VARIANT paths are not read as dataset references - Cross-check qualifiers against declared dataset names, so local aliases, CTEs and subquery sources are not reported as cross-dataset references - Extract a single cached _parse_expression helper, replacing two duplicated parse paths that had begun to diverge - Sort collision output for determinism under hash randomisation - Handle an explicitly null expression without raising AttributeError Spec and docs: - Metric Scoping promoted to a top-level section so its TOC entry resolves - Aggregation grain stated, and explicitly not dependent on primary_key - New rule: a dataset-scoped metric name must not collide with a field name of the same dataset - Namespace model documented, so the permitted repetitions are stated rather than left for implementations to constrain differently - docs/index.md and converters/README.md corrected; both said metrics are model-level only - Consumer guidance strengthened: ignoring datasets[].metrics is a lossy conversion, not a valid one The TPC-DS example is reverted to its state on main. No converter reads datasets[].metrics yet, so a flagship example using the placement would be lossy through every converter. Assisted-by: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Rule 1 originally read "a dataset-scoped metric's expression MUST NOT reference a field of another dataset", justified on the grounds that such a metric could then be exchanged without resolving the surrounding join graph. That justification is a claim about evaluation, and this specification does not define evaluation semantics: there is nothing in core-spec/ about how a metric is computed, how grain is resolved, or how fan-out is handled. Constraining authors on the strength of a benefit the spec never specifies overreaches. Raised by willpugh in review. Restating it as a placement rule removed the overreach but left the rule redundant. It is entailed by the unqualified-reference rule: an expression restricted to unqualified field names cannot reach another dataset, because a cross-dataset reference requires a qualifier. The rule stated a consequence and then needed two paragraphs to walk back the evaluation reading it invited, so it is now dropped and the consequence folded into rule 1 as a single clause. The remaining rules are renumbered 1 to 5. Also drops the aggregation grain paragraph and the corresponding comparison table row, for the same reason the rule 1 rationale went: grain is an evaluation concept, and this spec defines structure and naming rather than evaluation semantics. No check changes behaviour. Both previously documented INVALID snippets are still rejected; they are now both rule 1 violations. Assisted-by: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Matches the rest of spec.md, which uses one em dash in total. Assisted-by: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Rule 1 said a dataset-scoped metric's expression MUST reference fields of its declaring dataset. That forces an author to declare a field for a column they only want to aggregate, and then hide it. The expression is written against the dataset's source and references its columns, so nothing needs declaring first. This is not a new rule: it is how a field's own expression already works. The spec declares a field customer_id with expression customer_id, which refers to the source column rather than to the field itself, since referring to the field would be circular. That also settles a resolution question the old wording left open. A declared field does not shadow a source column of the same name, so if a dataset declares a field foo whose expression is not simply foo, then SUM(foo) in a metric of that dataset still refers to the source column. Reusing a declared field's expression inside a metric is the same composability question as a metric referencing another metric, and is left to a separate proposal. No check changes behaviour. validate_metric_scoping keys on dataset names and never on whether a leaf is a declared field, so permitting undeclared columns costs nothing mechanically. Two tests lock the documented behaviour in; both also pass against the previous validator, because it did not check field declarations either. Assisted-by: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
Rule 1 said a dataset-scoped metric's expression references fields of its dataset. That forces an author to declare a field for a column they only want to aggregate, and then hide it. It also narrowed something the spec already states: the Fields section describes fields as "row-level attributes that can be used for grouping, filtering, and in metric expressions". The expression now reaches both namespaces, with a distinct spelling for each. A declared field is written dataset_name.field_name, which is how a metric reuses a field's expression instead of repeating it. A column of the source is written unqualified. Two spellings rather than one shared namespace avoids a shadowing rule. A field and a source column may share a name without ambiguity, so declaring a field named after an existing column does not change the meaning of an expression already using the bare name. This replaces the hard error on self-qualification, since orders.amount now means the declared field amount, with a check that a qualified reference names a declared field of the declaring dataset. That is verifiable from the model, so SUM(orders.tax) is reported and points the author at SUM(tax). Whether a bare name is a real column stays unchecked, because it needs catalog metadata the model does not carry, and nothing else in validate.py checks a field's expression against real columns either. validate.py: _leading_qualifiers becomes _qualified_references, returning (qualifier, name) pairs so the name a qualifier introduces can be resolved against the dataset's field list. spec.md: the worked example now demonstrates both spellings on a derived field rather than two identity fields, and the second INVALID case is a qualified reference to an undeclared name instead of a self-qualifier. Tests: 21 functions added overall, collecting 32 cases with the 10 from apache#330. Reverting only validate.py to 592db69 fails 11 of the added functions and none of the 10 pre-existing ones. Assisted-by: Cortex Code <[email protected]> Co-authored-by: Isaac <[email protected]>
…ntics Split the two placements by where a measure belongs and tighten the dataset-scoped reference model: - Dataset-scoped metrics semantically belong to one dataset. Their expressions reference the dataset's declared fields by single-part (unqualified) name and resolve entirely within it, evaluated at the dataset's row level. Referencing a source column that is not a declared field is called out as a future extension. - Model-scoped metrics are for aggregations that need cross-dataset modeling: multi-part dataset.field references that combine fields across datasets via relationships. - Collapse the uniqueness rules to the one case this feature introduces: a dataset's field names and metric names must not collide. - Drop the consumer-flattening guidance, the name-repeat table, and the converter read-both / hoisting prose. - Reconcile the stale semantic_model.metrics references to the wrapper-less model root. Co-authored-by: Isaac <[email protected]>
…ping Teach the reference validator about dataset-scoped metrics: - validate_unique_names: a dataset's field names and dataset-scoped metric names share one namespace. Reject duplicate dataset-scoped metric names and any name used by both a field and a metric of the same dataset. - validate_metric_scoping (new): a dataset-scoped metric expression must reference the dataset's declared fields by single-part (unqualified) name. Reject qualified references (dataset.field) -- a self-qualification or a cross-dataset reference, which belong in the model-root metrics -- and references to any undeclared name (e.g. a raw source column). COUNT(*) and literals reference no field and are fine. Degrades gracefully for dialects sqlglot cannot parse. - validate_sql: run dataset-scoped metric expressions through the existing SQL-syntax check, alongside fields and model-scoped metrics. Tests cover each case in validation/tests/test_validate.py. Co-authored-by: Isaac <[email protected]>
kayemkim
left a comment
There was a problem hiding this comment.
Two tests fail with this branch merged onto current main (9531429), on every Python version in the matrix.
validation/tests/test_ontology.py::test_split_flights_examples_match_embedded_example compares examples/flights.semantic_model.yaml with the copy embedded in examples/flights.yaml, and the new total_seats metric on AIRCRAFT is only in the split file. python/tests/test_serialization.py::test_the_sample_document_sets_every_declared_field reports datasets[0].metrics missing from _fully_populated_document(), which that test asks to be extended whenever a property is added to the models. Both run in validation-ci, and the second also in python-ci.
The two new checks also disagree on whether names are case-sensitive. validate_metric_scoping casefolds, so a field Amount satisfies SUM(amount), while the namespace check in validate_unique_names compares exact strings, so a field Amount next to a dataset-scoped metric amount passes as two names. A document with both validates today. Nothing else in validate.py folds case and the Metric Scoping text does not say, so whichever rule is intended, the two checks and the spec sentence should agree.
On the parsing, _parse_metric_expression is a second bare-then-SELECT parse of the same expression, which is the mirroring JB asked to avoid on the list. validate_sql_expression already performs that parse and only drops the tree. If it returned the tree, or both went through one helper that returns the tree or the error, the statement handling from #527 and any later parser fix would reach the scoping check without a second copy.
…pache#591) Addresses the review feedback from @kayemkim on apache#591. Relocate the dataset-scoped metric example off the duplicated flights example pair onto the standalone tpcds model, cover the new dataset metrics property in the serialization fixture, and consolidate the SQL expression parse so the scoping check reuses one parser. - examples: remove AIRCRAFT.total_seats from flights.semantic_model.yaml so the split file again matches its embedded copy in flights.yaml, and add store_sales.total_quantity (SUM(ss_quantity)) to tpcds_semantic_model.yaml, which no test duplicates. This fixes test_split_flights_examples_match_embedded_example. - test: set datasets[].metrics in _fully_populated_document() so the OssieDataset.metrics property is exercised. This fixes test_the_sample_document_sets_every_declared_field. - validation: add _parse_sql_expression, which parses an expression once and returns the tree and any error. validate_sql_expression is now a thin wrapper over it, and validate_metric_scoping reuses it instead of a second bare-then-SELECT parse. This removes _parse_metric_expression. Co-authored-by: Isaac <[email protected]>
@kayemkim thanks for the review
|
|
@jbonofre when you have a chance, can you take a review pass. This should reflect the consensus in the thread here: https://lists.apache.org/thread.html/88vtqn72p15cyd9kjbbp3sp0pn5wgvgn |
Implement datasets[].metrics from apache/ossie#591 ahead of the schema, so models written against the next spec load on release. A dataset-scoped metric references its own declared fields by bare name; qualified names, sibling metrics and undeclared names are each refused. Fields and metrics share one namespace per dataset. A query names one as dataset.metric, which is also its output column. A model-scoped metric may build on one as dataset.metric, resolved by substitution like other metric references, so the result is the metric written out over fields and the fan-out check still applies. Neither #591 nor the spec defines this yet; ADR-0014 records the bet. ADR-0003 no longer attributes the bare-name metric reference to the schema, which does not contain it. A conformance test fails once the vendored schema gains the property, to reconcile the two.
Resolve converters/README.md modify/delete conflict by accepting upstream's deletion: upstream PR apache#523 (ae7d23f) moved the entire converters/ directory to apache/ossie-converters. The dataset-scoped metric documentation this branch had added to that README is preserved in core-spec/spec.md (Metric Scoping section), which is unaffected. Co-authored-by: Isaac <[email protected]>
|
@jbonofre just rebased the pr to reflect the converter merge. will file a followup issue to update the converter readme once this change is merged |
Add compliance/query/relational_semantics.sql, a DuckDB-runnable file of 29 test cases for core-spec/relational_semantics.md over the SCOTT schema. Each case pairs an Ossie query that uses AGG() (commented out) with an equivalent DuckDB query and its expected result. - Part A (12 cases): AGG() on one relation: grouping, filters, HAVING, NULL and empty-input values, ROLLUP, CTE propagation, subqueries. - Part B (17 cases): joins, including fan-out and many-to-many joins, and two queries run with each join type. The DuckDB queries use the reference evaluation strategy of Section 9. The model scott_model (commented out) uses dataset-scoped metrics (apache#591). The Ossie queries refer to its relations by two-part name, for example scott_model.emp. This is a light version of the .ost test format (apache#501) and view expander (apache#502). Co-authored-by: Isaac <[email protected]>
jbonofre
left a comment
There was a problem hiding this comment.
LGTM!
Declaring [c_customer_id] as an alternate unique key on customer restores converter-test coverage for non-primary-key unique keys (#519 removed this from the TPC-DS example).
This is an example and fixture change, no spec impact, so it passes by lazy consensus (as discussed on the mailing list).
Main's apache#591 added dataset-scoped metrics, which use unqualified field names. Update the Name Resolution section, expression_language.md, the Metrics intro, the Version History entry, and the spec.yaml comment so they distinguish dataset-scoped from model-scoped metric references. Co-authored-by: Isaac <[email protected]>
…ips, and metrics (#486) * [core-spec] Clarify name resolution for fields, keys, relationships, and metrics Metric expressions, primary_key, unique_keys, and relationship from_columns/to_columns refer to dataset fields, not columns of the dataset's source. Field expressions refer to source columns. Metric expressions must use qualified dataset.field references. Adds a Name Resolution section to spec.md, updates expression_language.md, schema descriptions, and spec.yaml comments, and fixes examples whose keys referenced undeclared fields. Co-authored-by: Isaac <[email protected]> * [core-spec] Cover dataset-scoped metrics in name resolution rules Main's #591 added dataset-scoped metrics, which use unqualified field names. Update the Name Resolution section, expression_language.md, the Metrics intro, the Version History entry, and the spec.yaml comment so they distinguish dataset-scoped from model-scoped metric references. Co-authored-by: Isaac <[email protected]> --------- Co-authored-by: Isaac <[email protected]>
Brings in the merged apache#591 (dataset-scoped metrics), apache#486 (name resolution), apache#614 and apache#619. Resolved conflicts: - python/src/ossie/models.py: union the pydantic imports (model_validator + SerializationInfo + TypeAdapter). - core-spec/spec.yaml, core-spec/spec.md: keep both the portable expression/dialects presence note and the upstream name-resolution notes; they are complementary. - validation/validate.py (semantic, not a text conflict): the shared _parse_sql_expression from apache#591 returns a (tree, error) tuple, so the unknown-dialect skip must return (None, None), not a bare None. This also makes validate_metric_scoping treat an unknown dialect as opaque. Co-authored-by: Isaac <[email protected]>
apache/ossie#486 settles that primary_key, unique_keys, from_columns and to_columns name fields of their dataset, not columns of its source. The loader checks them against the declared fields; joins, dictionary keys and semi-joins read the fields' expressions, so a key field may be named unlike its column or be an expression. A relationship or a key goes with its fields when the model is trimmed for a user. apache-ossie is pinned to 7205e15, which carries that commit. Its SDK also reads metrics declared on a dataset (apache/ossie#591); a model with one is refused until the planner answers them. The TPC-DS fixture takes the example's ss_ticket_number field and c_customer_id unique key. Co-Authored-By: Claude Opus 5.5 <[email protected]>
apache/ossie#486 settles that primary_key, unique_keys, from_columns and to_columns name fields of their dataset, not columns of its source. The loader checks them against the declared fields; joins, dictionary keys and semi-joins read the fields' expressions, so a key field may be named unlike its column or be an expression. A relationship or a key goes with its fields when the model is trimmed for a user. apache-ossie is pinned to 7205e15, which carries that commit. Its SDK also reads metrics declared on a dataset (apache/ossie#591); a model with one is refused until the planner answers them. The TPC-DS fixture takes the example's ss_ticket_number field and c_customer_id unique key. Co-Authored-By: Claude Opus 5.5 <[email protected]>
apache/ossie#591 lets a dataset declare metrics over its own fields, asked for as dataset.metric. They load, answer questions, filters and order_by, and aggregate their own dataset's rows, COUNT(*) included. A field and a metric of one dataset may not share a name; a dataset's metric that names anything but its own fields by bare name is refused in every question and reported by validate. Access trimming, the MCP views and the CLI list both levels through model.all_metrics. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Summary
NB: This PR is joint work with @jklahr. As called out under "Related Issues," this reflects
and update to #343 to incorporate the thread's consensus
Adds dataset-scoped metrics to the spec: a metric may now be declared on an
individual dataset (
datasets[].metrics) in addition to the model root(
metrics), using the identical metric structure in both places.references that dataset's declared fields by single-part (unqualified)
name (e.g.
SUM(amount)) and resolves entirely within the dataset, so thedataset carries its own measures and stays queryable on its own.
multi-part
dataset.fieldreferences and may combine fields from more thanone dataset.
Why: metrics that semantically belong to one dataset should live with it,
rather than all metrics being forced to the model level.
What's in the PR:
core-spec/spec.md(rules,comparison table, valid/invalid examples), plus the
datasets[].metricsproperty and reference comments in
core-spec/spec.yamlandcore-spec/ossie-schema.json.OssieDataset.metricsinpython/src/ossie/models.py.docs/index.md(overview, FAQ, glossary).converters/README.mdmapping notes (no convertercode changes; new converters live in
ossie-convertersper CONTRIBUTING).validation/validate.py+ tests:(no duplicates, no field/metric collisions);
the dataset's declared fields — qualified references and undeclared names
are errors.
Backward compatibility: additive and backward-compatible.
datasets[].metricsis a new optional property; existing documents and model-scoped metrics are
unaffected. No breaking changes.
Deliberately out of scope (future extension): a dataset-scoped metric
expression referencing a raw
sourcecolumn that is not a declared field. Thevalidator rejects it today; relaxing this later would be backward-compatible.
Attribution: the specification and Python-model design are the work of
Josh Klahr (@jklahr), carried over from #343 with his commits preserved
verbatim. The follow-on commits refine the wording per dev@ feedback (single-part
names, dataset-vs-model framing) and add the reference validator and its tests.
Related Issues
[email protected]— [DISCUSS] Dataset-scoped metrics(https://lists.apache.org/thread/8d9vg0pcwmx2tszzl8cn7r4mnc0mdkw2), including
the request to make qualified references an error and add a conformance test.
Checklist
Specification
core-spec/and follow the existing structureValidation
validation/are updated for the spec changeDocumentation
docs/is updated to reflect user-facing changesExamples
examples/updated —tpcds_semantic_modeldemonstrates a dataset-scoped metricTests
pytest/ CI green) — relying on PR CI; not run in the authoring environmentCompliance
AI disclosure
Per the ASF Generative Tooling Guidance, this contribution was prepared with AI
assistance. I have reviewed and verified every change.