Skip to content

fix(databricks): read OSSIE_SQL_2026 metric expressions - #446

Merged
jbonofre merged 1 commit into
apache:mainfrom
christianeu-db:fix/databricks-ossie-sql-2026
Sep 23, 2026
Merged

fix(databricks): read OSSIE_SQL_2026 metric expressions#446
jbonofre merged 1 commit into
apache:mainfrom
christianeu-db:fix/databricks-ossie-sql-2026

Conversation

@christianeu-db

@christianeu-db christianeu-db commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

#439 added OSSIE_SQL_2026 to the core spec and #440 added it to the Python
OssieDialect enum. This PR adds awareness to the Databricks converter.
pick_expression() only tried DATABRICKS and then ANSI_SQL, so a field or
metric written solely in OSSIE_SQL_2026 fell through to None and got dropped
from the Metric View.

OSSIE_SQL_2026 is ANSI-SQL-compatible, so this adds it to the fallback chain:

  • _common.py gains a DIALECT_OSSIE_SQL constant and pick_expression() now
    tries DATABRICKS, then ANSI_SQL, then OSSIE_SQL_2026.
  • The two drop warnings in ossie_to_metric_view.py mention the new dialect.
  • Four regression tests cover a field-only expression, a metric-only expression,
    the precedence order when several dialects are present, and an
    unsupported-dialect field that is still dropped with a warning.

metric_view_to_ossie.py only ever writes the DATABRICKS dialect on the way
out, so the reverse direction is untouched.

On the precedence order

OSSIE_SQL_2026 is currently last. The Databricks-native dialect should still
win where it exists, and ANSI_SQL is the established fallback. Once more of the
OSSIE_SQL_2026 expression-language semantics land as the canonical portable
form, the plan is to move it to the front of the chain so it becomes the
preferred source. This PR keeps that a one-line reordering rather than a behavior
change today.

This is a partial fix. #442 lists five call sites across the converters and
validation; this one covers the Databricks converter. #443 handles orionbelt.

Related Issues

Part of #442.

Checklist

Sections that this change does not touch (Specification, Ontology, Validation,
Documentation, Examples) are left unchecked.

Converters

  • Converter logic in converters/ is updated to reflect spec or ontology changes
  • New converters include tests under the converter's test directory

Tests

  • All existing tests pass (pytest / CI green)
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files
  • No third-party dependencies are added without PMC/IPMC approval

This pull request and its description were written by Isaac.

@christianeu-db Isaac
fix(databricks): read OSSIE_SQL_2026 metric expressions
c4db2e7 
OSSIE_SQL_2026 (core-spec apache#439, Python enum apache#440) is an ANSI-SQL-compatible
portable dialect, but the Databricks converter's pick_expression() fell back
only through DATABRICKS -> ANSI_SQL. A field or metric expressed only in
OSSIE_SQL_2026 returned None and was dropped from the Metric View output.

Add OSSIE_SQL_2026 to the fallback chain (DATABRICKS > ANSI_SQL >
OSSIE_SQL_2026), treating it as an ANSI_SQL-equivalent, and update the
drop-warning messages. Add regression tests covering a field-only and a
metric-only OSSIE_SQL_2026 expression plus the DATABRICKS preference order.

Partial fix for apache#442 (Databricks converter only).

Co-authored-by: Isaac <no-reply@databricks.com>
@christianeu-db
christianeu-db force-pushed the fix/databricks-ossie-sql-2026 branch from 4785339 to c4db2e7 Compare September 23, 2026 06:49
@christianeu-db
christianeu-db marked this pull request as ready for review September 23, 2026 06:50
@jbonofre
jbonofre self-requested a review September 23, 2026 07:13
@jbonofre
jbonofre merged commit 8eb5a21 into apache:main Sep 23, 2026
4 checks passed
@christianeu-db christianeu-db mentioned this pull request Sep 23, 2026
Merged
6 tasks done
Haoranli503 added a commit to Haoranli503/ossie_databricks that referenced this pull request Sep 23, 2026
@Haoranli503 Isaac
[OSSIE][DATABRICKS] Read OSSIE_SQL_2026 expressions on export
4b36719 
Port apache#446 to the Java converter. pickExpression only tried
DATABRICKS then ANSI_SQL, so a field or metric written solely in OSSIE_SQL_2026
(Apache Ossie's portable, ANSI-SQL-compatible dialect, added to the spec in
apache#439 and apache#440) fell through to null and was dropped from the Metric
View. Add OSSIE_SQL_2026 to the fallback chain (DATABRICKS, then ANSI_SQL, then
OSSIE_SQL_2026), update the two drop warnings, and add four regression tests
(field-only, metric-only, DATABRICKS-preferred precedence, unsupported-dialect
still dropped). The import direction only ever writes DATABRICKS, so it is
untouched.

Co-authored-by: Isaac <no-reply@databricks.com>
Haoranli503 added a commit to Haoranli503/ossie_databricks that referenced this pull request Sep 23, 2026
@Haoranli503 Isaac
Merge apache/ossie main into databricks-converter-v2
908d464 
Brings in apache#446 (OSSIE_SQL_2026 on the Python converter, already ported to Java on
this branch) plus other recent main work. Clean merge: git maps main's
converters/databricks/src edits onto this branch's moved converters/databricks/python
copy, so the Python converter stays in sync.

Co-authored-by: Isaac <no-reply@databricks.com>
@Haoranli503 Haoranli503 mentioned this pull request Sep 23, 2026
Merged
4 of 17 tasks
This was referenced Sep 25, 2026
Open
Open
jbonofre added a commit that referenced this pull request Sep 25, 2026
@Haoranli503 @jbonofre Isaac
[OSSIE][DATABRICKS] Add Java Converter and Reformat (#333)
6b682d9 
* [OSSIE] Add Java converter and restructure converters/databricks

Move the Python converter under converters/databricks/python/ (content unchanged) and add a Maven Java module -- library, CLI (OssieDatabricksConverter), JUnit tests, and fixtures under java/, package org.apache.ossie.converter.databricks -- as the maintained implementation. Add a root README describing the two-language layout, and a Java build job (mvn -B verify, JDK 21) in converter-databricks-ci.yml, mirroring the polaris converter.

Signed-off-by: Haoran Li <haoran.li@databricks.com>

* [OSSIE] Mark the Python converter as deprecated in favor of Java

Replace the Python README's forward-looking 'Future effort' section with a deprecation note: the Java converter under java/ is the maintained implementation; the Python copy is kept for reference and no longer actively extended.

Signed-off-by: Haoran Li <haoran.li@databricks.com>

* [OSSIE][DATABRICKS] Fix Java converter review findings

Build:
- maven-shade no longer writes dependency-reduced-pom.xml into the module
  root, where apache-rat failed `mvn verify` on it as an unapproved file
- configure surefire to include **/*Suite.java: the default includes match
  none of the test classes, so the build ran zero tests and still passed
- drop the **/*.md rat exclude and restore the ASF header on both READMEs
- align snakeyaml with the 2.3 that jackson-dataformat-yaml declares

Converter:
- qualifyMeasure matches the whole qualifier run and resolves it from the
  leaf, so an expression that already carries a join path is no longer
  qualified a second time (SUM(customer.customer.region.population))
- de-alias measure qualifiers on import, the inverse of the export rewrite
  and what resolveColumn already did for dimensions
- match dropped names outside string literals when cascading drops, so a
  name that only occurs in a literal no longer drops an unrelated column
- quoteReplacement the stash unicode-escape pass, which halved an escaped
  backslash run instead of re-emitting it verbatim
- notice the ai_context object members and the foreign-vendor extensions
  dropped from a field or a metric
- validate a join source on import with the rule the export applies, so a
  view that imports cleanly is always exportable again

CLI:
- name the directions from the Apache Ossie model's point of view, matching
  the library Javadoc and the Python CLI: export = Ossie -> Metric View
- give each command its own selector flag instead of sharing one field, and
  resolve the command before parsing arguments so --help prints usage
- print to stdout without the extra newline, so stdout and -o agree

Tests:
- generate join-qualified and nested-path measures, and one_to_many
  branches, in the property round-trip suites
- add a regression test per fix, plus a CLI suite (the CLI had none)

* Address internal bug bash feedbacks

Signed-off-by: Haoran Li <haoran.li@databricks.com>

* [OSSIE][DATABRICKS] Preserve measure display_name in the converter round trip

A dimension's display_name maps to the Ossie Field label, but the Ossie Metric schema has
no label, so a metric view measure's display_name was dropped in the MV -> Ossie -> MV
round trip. Preserve it in the DATABRICKS custom_extensions stash (the same mechanism as
format/window) in MetricViewToOssie.convertMeasure, and restore it in
OssieToMetricView.convertMetric.

Ports #326 (landed in Databricks runtime as databricks-eng/runtime#251390).

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Preserve non-equi join conditions via model-level complex_joins stash

A Metric View join whose `on` is not an equi-join of simple `alias.column` pairs (a non-equi
operator, a SQL-function-wrapped key, or an extra filter predicate) has no Apache Ossie
relationship form: the relationship schema requires from_columns/to_columns. The converter used
to abort the whole MV -> Ossie conversion on such a join.

Instead of aborting, preserve the join under the model's DATABRICKS custom_extensions
(complex_joins) and warn, rather than emitting a schema-invalid stub relationship with no columns.
The reverse converter merges the stashed joins back into the relationship graph and restores each
raw `on` verbatim, so such a metric view round-trips (nesting and one_to_many included).
Condition-less (cross) joins still have no Apache Ossie representation and are still rejected.

Also preserve an equi-join's original `on` verbatim when rebuilding it from the from/to columns
would not reproduce it (a fact side qualified by the source table name rather than `source`, or an
`on` over equal columns that would rebuild as `using`); canonical joins stash nothing.

Ports #321 (landed in Databricks runtime as databricks-eng/runtime#251398).

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Note measure display_name in the converter mapping tables

Upstream #356 clarified the mapping table (a measure's display_name has no
`label` on the Apache Ossie metric shape, so it rides in the DATABRICKS stash).
The converter restructure turned the top-level README into a short pointer, so
carry that clarification into the mapping tables now in python/README.md and
java/README.md. Both converters stash a measure's display_name (Python via #356,
Java via the measure display_name round-trip commit), so the tables match the
behavior.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Drop a measure naming a diamond-joined dataset with a notice

A measure addresses a dataset by name, so a bare reference to a dataset reached by more than one join path (a diamond) cannot be unambiguously qualified. Warn and drop it, mirroring how the dimension path handles a complex expression on a diamond, instead of silently binding to one arbitrary branch.

* [OSSIE][DATABRICKS] Bound export joins by MAX_JOIN_NODES to keep the round trip symmetric

The reverse conversion rejects a model with more than MAX_JOIN_NODES datasets, but the forward conversion had no matching bound, so a Metric View with too many joins could export to a model that could never be imported again. Reject it at export instead, with the same limit.

* [OSSIE][DATABRICKS] Keep the first DATABRICKS stash entry and warn instead of failing

A duplicate DATABRICKS custom_extensions entry is malformed input. Rather than reject the whole conversion, readStash now keeps the first entry, ignores the rest, and emits a notice so the dropped entry is not lost silently.

* [OSSIE][DATABRICKS] Update the Java README to match the converter behavior

Document the join-count bound on both conversion directions, the diamond-measure and duplicate-stash notices, and the model-level complex_joins stash for a non-representable join 'on' (dropping the stale line that listed non-equi joins as rejected).

* [OSSIE][DATABRICKS] Match cascade-drop references case-insensitively

Databricks SQL identifiers are case-insensitive, but cascade-drop matched
references to dropped fields and metrics case-sensitively, so a metric such as
COUNT(DISTINCT REGION_NAME) survived referencing a dropped region_name as a
dangling reference. Make both the pre-filter and the regex case-insensitive in
the propagation gate (matches) and the confirmation (referencesDropped), and
compile referencePattern with CASE_INSENSITIVE. Adds a test with the reported
repro.

Fixes #422.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Migrate the converter to flat model-at-root Ossie documents

Match apache/ossie #396: an Ossie document carries the model directly at the
document root (version, name, datasets, relationships, metrics) rather than
under a semantic_model wrapper. Import reads the root model and rejects a legacy
semantic_model wrapper, a missing string name, and root dialects/vendors; export
emits the model at the root. Migrates all test inputs and fixtures to the flat
format and re-roots the export assertions.

Verified by compiling the converter and running a driver (flat import, legacy
rejection, flat export, round trip) and converting all fixtures. The JUnit
assertions still need a mvn run as the final gate.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Put metrics at the model root in the cascade phase-order test

The flat-document migration left `metrics:` indented under the dataset item in
cascadeDropPreservesDimensionThenMeasurePhaseOrder, so `schemaMapList(model,
"metrics", ...)` returned empty and the metrics (m1/m0/bad_measure/keep) never
parsed. The test then saw 1 cascade notice instead of 4. Re-indent `metrics:`
and its items to column 0 (the model root). Verified against the compiled
converter: it now emits the 4 expected cascade notices in order.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Treat an empty explicit source as absent (truthy check)

An empty --source (for example an unset shell variable) was non-null, so it was
taken as a real override and failed as "requested source '' is not a dataset"
instead of falling back to the model source hint. Match the Python converter's
truthy check (explicit_source or ...): only a non-empty explicit source
overrides. Added a test that an empty source falls back exactly like an absent
one.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Treat an empty model name as absent on import (truthy check)

An empty --name (for example an unset shell variable) was non-null, so
MetricViewToOssie took it as a literal model name and emitted name: "" instead
of falling back to deriving the name from the source's last identifier. Match
the Python converter's truthy check (model_name or ...): only a non-empty name
overrides. Added a test that an empty name falls back exactly like an absent
one.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Read OSSIE_SQL_2026 expressions on export

Port #446 to the Java converter. pickExpression only tried
DATABRICKS then ANSI_SQL, so a field or metric written solely in OSSIE_SQL_2026
(Apache Ossie's portable, ANSI-SQL-compatible dialect, added to the spec in
#439 and #440) fell through to null and was dropped from the Metric
View. Add OSSIE_SQL_2026 to the fallback chain (DATABRICKS, then ANSI_SQL, then
OSSIE_SQL_2026), update the two drop warnings, and add four regression tests
(field-only, metric-only, DATABRICKS-preferred precedence, unsupported-dialect
still dropped). The import direction only ever writes DATABRICKS, so it is
untouched.

Co-authored-by: Isaac <no-reply@databricks.com>

* [OSSIE][DATABRICKS] Reject non-equi/unsupported join conditions on import

Match the Python converter (test_non_equi_on_rejected /
test_complex_equi_on_rejected): a join `on` that is not an equi-join of simple
`alias.column` pairs has no Apache Ossie relationship form (from/to columns are
required), so reject it on import rather than stashing it under the model's
DATABRICKS custom_extensions. Issue #321 asked for the preserve
behavior to be opt-in with the default unchanged; the Java port had made it the
unconditional default, diverging from Python. Remove the Java-only complex_joins
machinery on both the import (produce) and export (rebuild) sides -- Python has
no such concept -- and rework the round-trip tests into rejection tests.
Decomposable equi-joins, including a fact side qualified by the source table
name, are unaffected and still round-trip their `on` verbatim.

Co-authored-by: Isaac <no-reply@databricks.com>

---------

Signed-off-by: Haoran Li <haoran.li@databricks.com>
Co-authored-by: JB Onofré <jbonofre@apache.org>
Co-authored-by: Isaac <no-reply@databricks.com>
jbonofre pushed a commit that referenced this pull request Sep 25, 2026
@RyutoYoda
fix(dbt): read OSSIE_SQL_2026 expressions (#464)
ac54fbf 
* 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.

* fix(dbt): apply the dialect preference in _find_dataset_for_col too

Review follow-ups on #464.

_find_dataset_for_col still read field.expression.dialects[0] positionally,
so it and _get_expression could resolve the same field to different text and
a metric could be attributed to the wrong dataset. It now goes through
_get_expression, which makes it an instance method; its only call site
already went through self.

A dialect may also be listed more than once, since Expression.dialects has
minItems but no uniqueItems. The first OSSIE_SQL_2026 entry now wins, as the
converter's own dialect already does. The loop is not cut short on that match
because the converter's dialect outranks OSSIE_SQL_2026 and may appear later
in the list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbonofre jbonofre jbonofre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@christianeu-db @jbonofre