[core-spec] Clarify name resolution for fields, keys, relationships, and metrics - #486
justint-db wants to merge 2 commits into
Conversation
…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 <no-reply@databricks.com>
Use the spec.md wording for primary_key, unique_keys, from_columns, and to_columns in ossie-schema.json and spec.yaml, and drop the repeated view/compilation note from expression_language.md. Co-authored-by: Isaac <no-reply@databricks.com>
|
The following is the scenario I have been struggling with when creating the ThoughtSpot convertor. This prompted this initial post regarding the nameing conversion. In ThoughtSpot our most common architecture is two tiered. Where the following is an example. Convertor Logic. The solution I was going to land on (which raised this issue) was to Therefore the Semantic Layers metric definitions are at columns from layer 2. These do not necessarily represent what is defined in the database. I can convert a TS model to OSSIE and then convert the OSSIE model to DBX. However the database columns are never defined anywhere. To make this lossless should I export two definitions |
kayemkim
left a comment
There was a problem hiding this comment.
Ran this merged onto current main (edbcf79) with the workflows the changed paths trigger. Everything passes except Omni: test_tpcds_export_matches_expected fails on all five Pythons. The test reads examples/tpcds_semantic_model.yaml directly, and the new ss_ticket_number field now comes out as a visible dimension with its description, where converters/omni/tests/fixtures/tpcds_omni/views/store_sales.view.yaml still has the hidden: true dimension Omni synthesised for the undeclared key. That one entry is the only difference. The PR title check also rejects the [core-spec] prefix, it wants docs(core-spec): ... or Core-spec: ....
One data point for the breaking-change list. #485, merged yesterday for #466, moved the ThoughtSpot to-ossie direction the other way: on current main, a model whose display names differ from the warehouse names comes out with fields customer_id and id but from_columns: [cust_id_fk], to_columns: [cust_pk] and primary_key: [cust_pk], so its relationships and keys no longer name fields of the dataset. Under this text that converter joins the Databricks, dbt and NVIDIA group, and #466 probably wants reopening or a follow-up. For sequencing, #343 lets a dataset-scoped metric reference source columns unqualified, so the table here would want a row for datasets[].metrics when that lands.
The text follows the thread's conclusion closely, and the tie-break sentence plus the invalid-reference examples are the two things a converter author would otherwise have to ask about. LGTM on the spec side.
|
+1 on the resolution, and thank you for writing it down — "they never reach @djwaldo — on the two-dialect question, I think this PR already answers it, Your three levels line up with the rules here one-for-one:
That is the same shape as the worked example in this PR, where I would be wary of carrying it in two dialects. One note on the deferred list, specifically field-to-field references, since Ours are a dotted logical path resolved against declared fields The cost worth planning for is that the feature makes reference cycles On the tie-break rule — good that it is explicit. Since a field and a source For context, since this is my first comment here: I am CTO at XSOLCORP KOREA. Happy to review further revisions, particularly on round-trip and validation. |
Summary
The spec doesn't say whether a name like
amountin the metric expressionSUM(orders.amount)refers to a field declared in theordersdataset or to a column of the warehouse table named by itssource. Converters currently disagree (see the dev@ thread "what isamount?" started by Damian Waldron). The same ambiguity applies toprimary_key,unique_keys, and relationshipfrom_columns/to_columns.This PR follows the conclusion of that thread, which mirrors SQL views. A dataset's fields are defined in terms of its source columns, and model-level constructs are defined in terms of dataset fields. They never reach through a dataset to its source columns.
Related issues: #462 (metric expressions), #466 (relationship keys).
Changes
core-spec/spec.md: new Name Resolution section that sets out:expressionrefers to a source column of its own dataset.primary_key,unique_keys,from_columns, andto_columnsrefer to dataset fields.expressionrefers to a dataset field, which must be written asdataset.field.o_totalpriceis exposed astotal_amount), with examples of invalid references.core-spec/spec.md, other sections:core-spec/expression_language.md: the Name Spaces section now gives the rules for field and metric expressions instead of pointing to a separate document.core-spec/ossie-schema.json,core-spec/spec.yaml: only descriptions and comments changed. The schema itself is unchanged.examples/tpcds_semantic_model.yaml: added thess_ticket_numberfield, which thestore_salesprimary key and unique key already referenced.Breaking change
Earlier versions didn't say how these names resolve. This PR does, and marks the change Breaking in Version History:
dataset.fieldreferences. Unqualified field names are invalid.Converter test fixtures may need to be updated to follow these rules. A follow-up could add compliance fixtures where physical and logical names differ, as JB suggested on the thread.
AI disclosure
Per the ASF Generative Tooling Guidance, this contribution was prepared with AI assistance. All specification decisions and design choices are mine. I have reviewed and verified every change.