Skip to content

fix(orionbelt): export metric expressions that compute what OrionBelt computes - #472

Merged
jbonofre merged 2 commits into
apache:mainfrom
ralfbecher:fix/orionbelt-portable-export
Sep 29, 2026
Merged

jbonofre merged 2 commits into
apache:mainfrom
ralfbecher:fix/orionbelt-portable-export

Conversation

@ralfbecher

Copy link
Copy Markdown
Contributor

Summary

Other Ossie consumers (the Databricks, Snowflake and dbt converters, BI tools) read a metric's SQL expression, not the ORIONBELT extension. The OrionBelt converter exported SQL that did not compute what OrionBelt computes:

Before After
SUM(fact_orders.amount): physical table, not the dataset SUM("Orders"."amount"): <dataset>.<field>, always double-quoted
Measure filters dropped from the SQL SUM(CASE WHEN <condition> THEN <arg> END)
total: true dropped Grand-total window SUM(SUM(x)) OVER () (exact SUM/COUNT ratio for AVG)
{[Order Count]} and metric-on-metric refs left unresolved Synthesized counts and referenced metrics inlined
Period-over-period exported as x / NULLIF(prev.value, 0) - 1 (prev undefined) Left out with a warning, kept whole in the model extension
Cumulative/window ORDER BY the raw date Ordered and partitioned by the dimension at its timeGrain, as the query groups it

A measure or metric with no faithful single expression (period-over-period, grain, filterContext, anchor, window over a window, metric reference cycles) is left out of metrics with a warning and kept in the model-level extension (obml_unexported).

The rendering lives in a new _portable.py. It mirrors the OrionBelt compiler and adds no dependencies.

Round trip: every exported measure/metric carries its OBML definition (obml_definition), and every field its OBML column name (obml_column_name). OBML to Ossie to OBML now restores the original model instead of re-parsing filters and totals out of the SQL, where they would be applied twice. Column keys come back under their OBML names, not the physical codes, so several existing test assertions changed accordingly.

aggregation: measure exports MEASURE("<name>") tagged DATABRICKS instead of ANSI_SQL.

These fixes shipped in OrionBelt's in-repo copy of this converter first (ralforion/orionbelt-semantic-layer#480). They were reviewed there and verified against DuckDB.

Note: #410 (datatype in both directions) also edits obml_to_ossie.py and ossie_to_obml.py. Whichever lands second needs a small rebase. I'll do it.

Related Issues

None filed. Found while testing the OrionBelt to Ossie to Databricks Metric View chain.

Checklist

Converters

  • Converter logic in converters/ is updated
  • Tests under converters/orionbelt/tests/: new test_ossie_portable_expressions.py, which runs every exported metric in DuckDB grouped as OrionBelt groups it (skipped when duckdb/sqlglot are not installed)

Documentation

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

Tests

  • pytest in converters/orionbelt: 186 passed, 3 skipped (189 passed with duckdb and sqlglot)
  • ruff check / mypy report nothing new

Compliance

  • ASF license headers on the new source and test files
  • No third-party dependencies added

… computes

Other Ossie consumers read a metric's SQL, not the ORIONBELT extension. That
SQL referenced the physical table instead of the dataset, dropped measure
filters and totals, left {[Name]} references to counts and metrics unresolved,
and exported period-over-period as prev.value.

Export (new _portable.py renderer):
- reference "<dataset>"."<field>", always double-quoted so reserved words parse
- filters as AGG(CASE WHEN ... END), totals as the grand-total window
  SUM(SUM(x)) OVER () (exact SUM/COUNT ratio for AVG), defaultValue as
  COALESCE, count_distinct as COUNT(DISTINCT ...)
- inline synthesized counts and metric-on-metric references
- cumulative and window metrics order and partition by each dimension at its
  timeGrain, cast back to a temporal resultType, as the query groups it
- leave out period-over-period, grain/filterContext/anchor measures, nested
  windows and reference cycles with a warning, kept whole in the model
  extension (obml_unexported)
- tag aggregation: measure as DATABRICKS
- carry each column's OBML name and each measure/metric's OBML definition

Import:
- restore definitions, left-out entities and column names, so
  OBML -> Ossie -> OBML returns the model it was given instead of
  re-parsing filters and totals out of the SQL

New tests run every exported metric in DuckDB, grouped as OrionBelt groups it
(skipped when duckdb or sqlglot is not installed).
jbonofre
jbonofre previously approved these changes Sep 29, 2026

@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 (just a very minor nit about import order in a test).

Thanks @ralfbecher !

import json
from typing import Any

import pytest

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.

Nit: running ruff check on the new test file raises an I001 (isort) violation due to the import order of pytest and ossie_orionbelt.

I suggest to run ruff check --fix on test_ossie_portable_expression.py to keep the linter clean.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I could reproduce it only with ruff before 0.6 (or --isolated): those versions do not look in src/ for first-party packages, so ossie_orionbelt sorts as third-party. That flags test_ossie_v02_compat.py, test_ossie_metric_no_silent_loss.py, test_ossie_tpcds_baseline.py and test_ossie_converter_cumulative.py the same way, while current ruff (0.15) passes all of them and would flag the reordered file instead.

So rather than reorder this one file, I pinned known-first-party = ["ossie_orionbelt"] under [tool.ruff.lint.isort] in converters/orionbelt/pyproject.toml. ruff check --select I now passes on both ruff 0.5.7 and 0.15.17, with no file changes.

Ruff before 0.6 does not look in src/ for first-party packages, so it sorted
ossie_orionbelt with the third-party imports and reported I001 on five test
files. Naming the package makes the import order the same on every ruff
version.
@jbonofre
jbonofre merged commit adfa9e4 into apache:main Sep 29, 2026
7 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.

2 participants