Skip to content

fix(orionbelt): name fields by column, quote only when needed, export OSSIE_SQL_2026 - #605

Merged
jbonofre merged 2 commits into
apache:mainfrom
ralfbecher:fix/orionbelt-field-names-flat-ossie-sql
Oct 20, 2026
Merged

jbonofre merged 2 commits into
apache:mainfrom
ralfbecher:fix/orionbelt-field-names-flat-ossie-sql

Conversation

@ralfbecher

@ralfbecher ralfbecher commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three fixes to the OrionBelt converter, measured with the #596 portability fixtures and the TPC-DS example.

1. A field keeps its name (#601). The export named each Ossie field by its OBML column code. After an Ossie import, the code of a calculated field is its SQL, so amount_with_tax (amount * 1.1) came back as a field named amount * 1.1. Fields are now named by the OBML column name, with the code as their expression, which is the distinction an Ossie field already makes. primary_key, unique_keys and relationship columns use the same names.

Documents written by earlier versions, which named fields by code and kept the column name in obml_column_name, still import. The unique keys and preserved metrics such a document carries are renamed with the fields. Only the field references in a metric's SQL change; a table or alias the SQL declares itself is left alone, and a reference that cannot be renamed safely is kept and warned about.

2. A reference is quoted only when it has to be (#576). Every metric reference was double-quoted. The expression language follows ANSI identifier rules (core-spec/expression_language.md, "Namespacing and Identifier Resolution"): a regular identifier is normalized to upper case, a quoted one is taken as written, so "sales"."amount" did not match a field declared amount. A name is now bare when it can be a regular identifier. A name that cannot (a space, a leading digit, a SQL:2023 reserved word such as Order or value) is quoted as written. A regular name DuckDB or sqlglot will not read bare, such as Qualify, is quoted in its normalized form, "QUALIFY", which matches it. Names that differ only by case are one identifier to the language; they are quoted as written, with a warning. The import resolves references by the same rules: a quoted reference exactly, else by its normalized form, a bare one regardless of case, and a metric whose reference matches no field or more than one is kept verbatim rather than resolved to the first. The ontology export's object and link mappings are written the same way, over the embedded model's dataset and field names.

3. Metrics also carry OSSIE_SQL_2026 (#576). The export wrote every expression as ANSI_SQL, so a metric imported in the spec's own language came back retagged. Each measure or metric now also carries an OSSIE_SQL_2026 expression, in that language's forms (CONTAINS, STARTSWITH, ENDSWITH instead of LIKE ... ESCAPE, a CAST for a timestamp literal). It is checked against core-spec/expression_language.md both as written (every function, CAST type and typed literal is one the spec lists at the REQUIRED or RECOMMENDED level, and there is no foreign operator such as ::) and as parsed (no construct the spec does not list). A measure with no such form (MODE, a pattern that needs ESCAPE, an authored expression outside the language) keeps ANSI_SQL only. A function sqlglot respells (VARIANCE_POP, STR_POSITION, CAST(x AS TEXT(2))) is written in the spec's spelling (VAR_POP, CHARINDEX, CAST(x AS STRING(2))), keeping a type's precision or length.

Also: an expression containing a query (a subquery or a UNION) is now preserved verbatim. Before, the aggregates inside the subquery were lifted into measures, even where the subquery's own alias shadowed a dataset name.

Round trip, Ossie to OBML to Ossie

Fixture main This PR
M1 m1_scalar_field amount_with_tax comes back named amount * 1.1 2/2 fields keep their names
TPC-DS example customer_full_name comes back named c_first_name || ' ' || c_last_name 31/31 fields keep their names
M0 m0_ossiesql_qualified retagged ANSI_SQL, text SUM("sales"."amount") OSSIE_SQL_2026 kept, text unchanged
M0, M2, M3 qualified text rewritten to "sales"."amount" text unchanged

On every fixture, each metric's text comes back unchanged and gains the other dialect. All nine returned documents pass validation/validate.py.

Dependency

This adds sqlglot>=30,<31 as a runtime dependency. Eight other converters already declare sqlglot (holistics, nvidia, snowflake, microsoft, dbt, cube, sigma, thoughtspot). The cap below 31 is deliberate: the OSSIE_SQL_2026 check depends on sqlglot's token types and on how it spells normalized functions.

Origin

These fixes shipped in OrionBelt's in-repo copy of this converter first (ralforion/orionbelt-semantic-layer#586, #587, #592, #594), where they were reviewed. The port is adapted to this copy: the import keeps its own metric parsing, and nested data objects, which this copy's OBML schema does not have, are left out.

Related Issues

Closes #601. Part of #576.

Checklist

Converters

  • Converter logic in converters/ is updated
  • Tests under converters/orionbelt/tests/: new test_ossie_field_names.py and test_ossie_sql_2026_dialect.py, plus cases in test_ossie_portable_expressions.py and test_ossie_metric_no_silent_loss.py

Documentation

  • converters/orionbelt/README.md and ossie_obml_mapping_analysis.md updated

Tests

  • pytest in converters/orionbelt: 625 passed, 5 skipped (the skips need duckdb, as before)
  • ruff check, ruff format --check and mypy report nothing new against main

Compliance

  • ASF license headers are present on all new source files (dev/check-license passes)
  • sqlglot (MIT) is declared as a dependency, not vendored

… OSSIE_SQL_2026

The export named each Ossie field by its OBML column code. After an Ossie
import that code can hold a calculated field's SQL, so amount_with_tax
(amount * 1.1) came back as a field named "amount * 1.1" (apache#601). Fields
are now named by the OBML column name with the code as their expression,
and primary_key, unique keys and relationship columns use the same names.
Documents from earlier versions still import; the unique keys and
preserved metrics they carry are renamed with the fields.

Metric references were always double-quoted, so "sales"."amount" did not
match a field declared amount under ANSI identifier rules. A name is now
quoted only when it cannot be a regular identifier or differs from
another only by case.

Each exported metric also carries an OSSIE_SQL_2026 expression where it
has a form in that language, checked against core-spec/expression_language.md,
so a metric imported in the spec's language keeps its tag (apache#576).

An expression containing a query is preserved verbatim instead of having
the subquery's aggregates lifted into measures.

Adds sqlglot as a runtime dependency, as eight other converters do.

field: dict[str, Any] = {
"name": code,
"name": col_name,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming fields by column name here breaks the ontology export, which this PR doesn't touch. OBMLtoOssieOntology still builds its mapping expressions from the table code and column codes (_table_ref, and _col_code in ontology.py). For a data object Orders with the primary key column Order ID (code order_id), it emits orders.order_id, but the embedded semantic model now has the field Order ID. The ontology spec defines mappings as SQL expressions over dataset fields, so none of the object or link mappings resolve any more. They all resolve on main.

I think ontology.py should build these expressions from the dataset name and the column name, quoted with ossie_ident, e.g. Orders."Order ID".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rename also changes dimension types in a round trip. On export, _extract_dimensions takes the dimension resultType straight from _infer_obml_type, a name heuristic that only matches keywords at _ boundaries.

When fields were named by code, unit_price matches price. Unit Price matches nothing, so OBML -> Ossie -> OBML turns a float dimension into resultType: string, and the same happens to Item Count (int). The column keeps its type because _convert_dataset checks datatype first.

Could the dimension use the same datatype-first lookup as the column? Moving those lines into a helper would cover both.

Comment on lines +118 to +119
for entry in (metric.get("expression") or {}).get("dialects") or []:
dialect = entry.get("dialect")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes expression is a dict and that each dialect entry is a dict. Preserved metrics are exactly the ones the converter could not handle, so they are the most likely to be malformed. An older (unmarked) obml_unconverted_metrics payload holding {"name": "m", "expression": "SUM(Orders.amount)"} now makes OBMLtoOssie.convert() raise AttributeError: 'str' object has no attribute 'get'. On main it passes through.

Going the other way, Ossie -> OBML, a legacy document with obml_column_name stashes and such a metric crashes the same way. The crash goes through the loop at ossie_to_obml.py, which also needs an isinstance(metric, dict) check.

Suggested change
for entry in (metric.get("expression") or {}).get("dialects") or []:
dialect = entry.get("dialect")
expression = metric.get("expression")
if not isinstance(expression, dict):
return
for entry in expression.get("dialects") or []:
if not isinstance(entry, dict):
continue
dialect = entry.get("dialect")

such as ``Order``) is quoted, and matches the declared name exactly, as is
a name in *collisions*, which bare would match another one.
"""
return sql_ident(name) if name in collisions or needs_quoting(name) else name

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting names that differ only by case (Orders."amount" vs Orders."Amount") relies on a quoted name matching exactly, and the rename_sql docstring says that's how the import resolves metric references. It doesn't: _resolve_column_refs strips the quotes and lower-cases the column, and fields_lc keeps only the first field for each lower-cased name (setdefault).

With fields Amount (expression amt) and amount (amt2), the metric SUM(Orders."amount") becomes a measure on Amount. The round trip comes back as SUM(Orders."Amount") and sums the wrong column without a warning.

The import should match a quoted column exactly and ignore case only for a bare one. A bare reference that matches two fields when case is ignored should be kept as an unconverted metric rather than resolved to the first field.

Related, and maybe worth settling first: if I read the spec's identifier rules right, an unquoted identifier is normalized to upper case, so a quoted "amount" would not match a field declared as amount either. That would change how these names should be written in the first place.

Comment on lines +71 to +75
try:
tree = sqlglot.parse_one(sql, read=_OSSIE_DIALECT_TO_SQLGLOT.get(dialect))
except sqlglot.errors.SqlglotError:
return False
return tree.find(exp.Query) is not None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When sqlglot can't parse the text, this returns False, and unlike _field_names._parse it doesn't retry with [name] brackets. So a bracket-quoted metric with a subquery still gets the subquery's aggregates lifted, which is what this check is meant to stop.

For example, SUM([Orders].[amount]) + (SELECT MAX([Orders].[amount]) FROM adjustments AS [Orders]) produces a _Orders_amount_max measure and the broken metric {[_Orders_amount_sum]} + (SELECT {[_Orders_amount_max]} FROM adjustments AS [Orders]).

Reusing _parse fixes this; it would need to be imported, or made public:

Suggested change
try:
tree = sqlglot.parse_one(sql, read=_OSSIE_DIALECT_TO_SQLGLOT.get(dialect))
except sqlglot.errors.SqlglotError:
return False
return tree.find(exp.Query) is not None
parsed = _parse(sql, _OSSIE_DIALECT_TO_SQLGLOT.get(dialect))
return parsed is not None and parsed[1].find(exp.Query) is not None

…p ontology over fields

Address the review of apache#605:

- The ontology export builds its object and link mappings from the dataset
  and field names, quoted as the model's metrics are, so they resolve against
  the embedded semantic model (`Orders."Order ID"`, not `orders.order_id`).
- A dimension takes its type from the field `datatype` first, like the
  column, instead of a guess from its name; a time dimension over a
  non-temporal field stays a date dimension.
- Renaming a preserved metric skips an expression or dialect entry that is
  not a mapping, instead of raising AttributeError.
- The subquery check retries a bracket-quoted expression as the rename does,
  so a bracketed metric with a subquery is preserved verbatim.
- References follow core-spec/expression_language.md: a regular identifier
  is normalized to upper case, a quoted one is taken as written. The import
  matches a quoted reference exactly, else by its normalized form, and a
  bare one regardless of case; a reference that matches no field or more
  than one keeps the metric verbatim instead of resolving to the first. The
  export quotes a regular name DuckDB or sqlglot will not read bare in its
  normalized form ("QUALIFY"), and warns of names that differ only by case.
ralfbecher added a commit to ralforion/orionbelt-semantic-layer that referenced this pull request Oct 18, 2026
…nsions by datatype (#599)

Mirror the review of apache/ossie#605 in the in-repo converter:

- References follow the Ossie expression language's identifier rules: a
  bare name is normalized to upper case, a quoted one is taken as written.
  The import matches a quoted reference exactly, else by its normalized
  form, and a bare one regardless of case; a reference that matches no
  dataset or field, or more than one, keeps the metric verbatim instead of
  resolving to the first. Databricks compares quoted names regardless of
  case, so there they are read as bare. The export quotes a regular name
  DuckDB or sqlglot will not read bare in its normalized form ("QUALIFY")
  and warns of names that differ only by case.
- An imported dimension takes its type from the field datatype first, like
  the column; a time dimension over a non-temporal field stays a date one.
- Renaming a preserved metric skips an expression or dialect entry that is
  not a mapping, instead of raising AttributeError.

@kayemkim kayemkim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a document the earlier export wrote, _restore_column_names renames the column keys, each dimension's column, the measure references, the unique keys and the preserved metrics, but not the dimension names themselves. Those come out of _extract_dimensions as the field name, which in such a document is the code or the expression. obml-to-ossie on main from tests/fixtures/tpcds_as_obml.yaml, then ossie-to-obml on this branch, gives dimensions a key c_first_name || ' ' || c_last_name with column: customer_full_name, where the fixture names that dimension customer_full_name. The same with a _legacy_field-shaped input: a field named amount * 1.1 that stashes Amount With Tax and carries dimension comes back with the column key Amount With Tax and the dimension key amount * 1.1, with no warning. Renaming the dimension keys in that pass would close it, and TestLegacyCodeNamedExport has no dimension assertion yet. Everything else in that cross-version trip matches the fixture: columns, codes, keys, joins, measures and metrics.

On d40a0a6, the M1 fixture keeps both field names, the TPC-DS example keeps 31 of 31 and examples/flights.semantic_model.yaml 51 of 51. Every metric in the eight #596 fixtures comes back with its text unchanged and the second dialect added, the OSSIE_SQL_2026 tag on m0_ossiesql_qualified survives, and each returned document passes validate.py. The bare-form fixtures still gain the helper measures they did on main, which this PR does not touch. The five points from the review behave as the second commit says with my inputs: Orders."Order ID" in the object mapping, Unit Price back as float and Item Count as int, a string-valued preserved metric passing through in both directions, SUM(Orders."amount") next to Amount resolving to amount while the bare reference is kept and warned, and the bracket-quoted subquery kept verbatim.

On the identifier question. As the branch stands, a declared regular name goes out bare and spelled as declared (sales.TotalAmount), so a reader that matches exactly and one that folds case both resolve it, and the import takes "TOTALAMOUNT", "TotalAmount" and a bare reference in any case. A reserved word (value, order, date, count) or a name that cannot be regular (Order ID, 2nd_col) goes out quoted as written, and Qualify as "QUALIFY". The one case where this and a validator could disagree is a declared name that is a reserved word: this branch treats it as delimited, while a validator that normalizes declared names by shape alone would upper-case it to VALUE and not match "value". That is the sentence the spec would need, and it does not block this PR.

@jbonofre
jbonofre self-requested a review October 19, 2026 06:37

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 👍

The ontology mappings, the datatype-first dimension types, the malformed preserved-metric handling, the exact quoted-name matching and the bracket-quoted subquery check all behave as intended now.

We could consider the follow-ups:

  • _restore_column_names renames column keys, dimension column, measure references, unique keys and preserved metrics, but not the dimension keys themselves. A document written by the previous version can still come back with a dimension named by its code or expression. TestLegacyCodeNamedExport could get a dimension assertion alongside the fix.
  • _convert_column no longer falls back to the column name as description or adds the display-name synonym. _extract_dimensions now forces date for an is_time dimension over a non-temporal field. Both are reasonable, but they aren't listed in the description.

@jbonofre
jbonofre merged commit b42ce09 into apache:main Oct 20, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OrionBelt converter renames a calculated field to its expression on the way back to Ossie

3 participants