Skip to content

Commit 7765997

Browse files
committed
fix(dbt): read OSSIE_SQL_2026 expressions instead of picking by position
OSSIE_SQL_2026 (#439, #440) was not recognised anywhere in converters/dbt. _get_expression() matched only self._dialect -- always ANSI_SQL, since the constructor argument is never passed -- and otherwise returned dialects[0]. Expression.dialects carries no ordering constraint, so an expression written in OSSIE_SQL_2026 alongside a vendor dialect resolved by array position: with the vendor entry first a Snowflake-only column reference was written into the MetricFlow output as portable SQL, with no warning and no ConverterIssue. expression_language.md asks implementations to always support the Ossie dialect and to choose deterministically between dialects; this did neither. Add OSSIE_SQL_2026 to the chain as an ANSI_SQL equivalent (self._dialect > OSSIE_SQL_2026 > dialects[0]), mirroring #443/#446/#447 for orionbelt, databricks and snowflake. The change is additive: expressions carrying ANSI_SQL keep their current behaviour. Add regression tests for the metric and field paths in either dialect order, for ANSI_SQL keeping precedence over OSSIE_SQL_2026, and for the positional fallback when no portable dialect is present. tests/helpers.py only builds single-dialect expressions, so this is the first multi-dialect coverage in the Ossie -> dbt direction. Partial fix for #461 (_get_expression only); the positional read at ossie_to_msi.py:401 is left for a follow-up.
1 parent 3ccd934 commit 7765997

2 files changed

Lines changed: 113 additions & 2 deletions

File tree

‎converters/dbt/src/ossie_dbt/ossie_to_msi.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,23 @@ def _find_dataset_for_col(
405405
return datasets[0].name if datasets else ""
406406

407407
def _get_expression(self, ossie_expr: OssieExpression) -> str:
408-
"""Return the expression string for the preferred dialect (fallback: first available)."""
408+
"""Return the expression string for the preferred dialect.
409+
410+
Preference order: the converter's dialect, then OSSIE_SQL_2026, then the
411+
first entry available. OSSIE_SQL_2026 is Ossie's portable expression
412+
language, based on ANSI SQL:2003 Core, so it is treated as an ANSI_SQL
413+
equivalent rather than left to the positional fallback.
414+
"""
415+
ossie_sql_expr: Optional[str] = None
416+
409417
for dialect_expr in ossie_expr.dialects:
410418
if dialect_expr.dialect is self._dialect:
411419
return dialect_expr.expression
420+
if dialect_expr.dialect is OssieDialect.OSSIE_SQL_2026:
421+
ossie_sql_expr = dialect_expr.expression
422+
423+
if ossie_sql_expr is not None:
424+
return ossie_sql_expr
412425
return ossie_expr.dialects[0].expression if ossie_expr.dialects else ""
413426

414427
@staticmethod

‎converters/dbt/tests/test_ossie_to_msi.py‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,16 @@
2020
import pytest
2121
from syrupy.assertion import SnapshotAssertion
2222

23-
from ossie import OssieDataType, OssieDimension
23+
from ossie import (
24+
OssieDataType,
25+
OssieDialect,
26+
OssieDialectExpression,
27+
OssieDimension,
28+
OssieDocument,
29+
OssieExpression,
30+
OssieField,
31+
OssieMetric,
32+
)
2433
from ossie_dbt.msi_to_ossie import MSIToOssieConverter
2534
from ossie_dbt.ossie_to_msi import OssieToMSIConverter
2635
from metricflow_semantic_interfaces.implementations.elements.measure import (
@@ -456,6 +465,95 @@ def test_percentile_cont_non_median_carries_percentile_param(self) -> None:
456465
assert m.type_params.expr == "amount"
457466

458467

468+
def _multi_dialect_expr(*pairs: tuple[OssieDialect, str]) -> OssieExpression:
469+
"""Build an expression carrying more than one dialect, in the given order."""
470+
return OssieExpression(
471+
dialects=[OssieDialectExpression(dialect=dialect, expression=expr) for dialect, expr in pairs]
472+
)
473+
474+
475+
_SNOWFLAKE_METRIC = (OssieDialect.SNOWFLAKE, "SUM(orders.amt_snowflake_only)")
476+
_OSSIE_SQL_METRIC = (OssieDialect.OSSIE_SQL_2026, "SUM(orders.amount)")
477+
_ANSI_METRIC = (OssieDialect.ANSI_SQL, "SUM(orders.amount_ansi)")
478+
479+
480+
def _doc_with_metric_expression(expression: OssieExpression) -> OssieDocument:
481+
return _ossie_doc(
482+
datasets=[_ossie_dataset("orders", fields=[_ossie_field("amount")])],
483+
metrics=[OssieMetric(name="revenue", expression=expression)],
484+
)
485+
486+
487+
class TestOssieToMSIDialectSelection:
488+
@pytest.mark.parametrize(
489+
"order",
490+
[
491+
(_OSSIE_SQL_METRIC, _SNOWFLAKE_METRIC),
492+
(_SNOWFLAKE_METRIC, _OSSIE_SQL_METRIC),
493+
],
494+
ids=["ossie_sql_first", "vendor_first"],
495+
)
496+
def test_ossie_sql_2026_wins_over_a_vendor_dialect_in_either_order(
497+
self, order: tuple[tuple[OssieDialect, str], ...]
498+
) -> None:
499+
doc = _doc_with_metric_expression(_multi_dialect_expr(*order))
500+
501+
result = OssieToMSIConverter().convert(doc).output
502+
503+
assert result.metrics[0].type_params.expr == "amount"
504+
505+
@pytest.mark.parametrize(
506+
"order",
507+
[
508+
(_ANSI_METRIC, _OSSIE_SQL_METRIC),
509+
(_OSSIE_SQL_METRIC, _ANSI_METRIC),
510+
],
511+
ids=["ansi_first", "ossie_sql_first"],
512+
)
513+
def test_ansi_sql_still_takes_precedence_over_ossie_sql_2026(
514+
self, order: tuple[tuple[OssieDialect, str], ...]
515+
) -> None:
516+
doc = _doc_with_metric_expression(_multi_dialect_expr(*order))
517+
518+
result = OssieToMSIConverter().convert(doc).output
519+
520+
assert result.metrics[0].type_params.expr == "amount_ansi"
521+
522+
def test_field_expression_prefers_ossie_sql_2026_over_a_vendor_dialect(self) -> None:
523+
doc = _ossie_doc(
524+
datasets=[
525+
_ossie_dataset(
526+
"orders",
527+
fields=[
528+
OssieField(
529+
name="region",
530+
expression=_multi_dialect_expr(
531+
(OssieDialect.SNOWFLAKE, "region_snowflake_only"),
532+
(OssieDialect.OSSIE_SQL_2026, "region_portable"),
533+
),
534+
)
535+
],
536+
)
537+
]
538+
)
539+
540+
sm = OssieToMSIConverter().convert(doc).output.semantic_models[0]
541+
542+
assert [(d.name, d.expr) for d in sm.dimensions] == [("region", "region_portable")]
543+
544+
def test_first_entry_is_still_used_when_no_portable_dialect_is_present(self) -> None:
545+
doc = _doc_with_metric_expression(
546+
_multi_dialect_expr(
547+
(OssieDialect.SNOWFLAKE, "SUM(orders.amt_snowflake_only)"),
548+
(OssieDialect.DAX, "SUM(orders.amt_dax_only)"),
549+
)
550+
)
551+
552+
result = OssieToMSIConverter().convert(doc).output
553+
554+
assert result.metrics[0].type_params.expr == "amt_snowflake_only"
555+
556+
459557
class TestOssieToMSIRoundTrip:
460558
def test_ossie_to_msi_to_ossie_preserves_structure(self, snapshot: SnapshotAssertion) -> None:
461559
"""Ossie → MSI → Ossie preserves dataset names, fields, and metric expressions."""

0 commit comments

Comments
 (0)