fix(Databricks): round-trip display_name on measures (#326) - #356
Merged
jbonofre merged 2 commits intoSep 3, 2026
Merged
Conversation
A dimension's display_name maps to the Apache Ossie field `label`, but the Apache Ossie metric shape has no `label`, so a measure's display_name was silently dropped in the MV -> Ossie -> MV round trip. Preserve it in the DATABRICKS custom_extensions stash (the same mechanism as format/window), kept in a separate MEASURE_STASH_KEYS list so the dimension path -- which already maps display_name to `label` -- does not also stash it. The exporter restores it in _convert_metric. Fixes #326. Co-authored-by: Isaac <no-reply@databricks.com>
christianeu-db
marked this pull request as draft
September 3, 2026 20:20
christianeu-db
changed the title
Contributor
Co-authored-by: Isaac <no-reply@databricks.com>
christianeu-db
marked this pull request as ready for review
September 3, 2026 20:32
jbonofre
self-requested a review
September 3, 2026 23:14
jbonofre
approved these changes
Sep 3, 2026
Member
|
Good catch! Thanks! |
Haoranli503
added a commit
to Haoranli503/ossie_databricks
that referenced
this pull request
Sep 4, 2026
…g tables Upstream apache#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 apache#356, Java via the measure display_name round-trip commit), so the tables match the behavior. Co-authored-by: Isaac <no-reply@databricks.com>
jbonofre
added a commit
that referenced
this pull request
Sep 25, 2026
* [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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Databricks converter dropped a measure's
display_nameduring theMV -> Ossie -> MVround trip. A dimension'sdisplay_namemaps to the Apache Ossie fieldlabel, but the Apache Ossie metric shape has nolabel, so a measure'sdisplay_namehad nowhere to go and was silently lost.This preserves it in the
custom_extensions[DATABRICKS]stash (the same mechanism already used forformatandwindow).Related Issues
Fixes #326 for the python converter. #333 will be augmented to include this fix for the Java converter.
Checklist
Specification
N/A - no spec changes
core-spec/and follow the existing structureOntology
N/A - no ontology changes
ontology/are consistent with spec changesConverters
converters/is updated to reflect spec or ontology changesValidation
N/A - no new validation rules
validation/are updated if the spec changedDocumentation
N/A no docs
docs/is updated to reflect any user-facing changesCONTRIBUTING.mdis updated if the contribution process changedExamples
N/A - no new examples
examples/are added or updated for any new spec constructs or converter supportTests
pytest/ CI green)Compliance
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.