From cf72ae5935cee8d7ada3330dbfcf8e5ff0c5bc65 Mon Sep 17 00:00:00 2001 From: Haoran Li Date: Thu, 1 Oct 2026 21:20:16 +0000 Subject: [PATCH 1/2] fix(databricks): tighten cascade-drop reference matching A dropped field or metric cascades to columns that reference it, but the match was too broad in two ways: - A bare dropped name matched a same-spelled SQL function call, so dropping a field named `year` also dropped a measure like `COUNT_IF(YEAR(d) = 2026)`. The dimension reference pattern now excludes a `NAME(...)` function call. - The self-reference guard was case-sensitive and not scoped to the column's kind, so a survivor differing from a dropped field only in case was dropped, while a measure sharing a dropped dimension's name was wrongly exempted. The guard is now case-folded and scoped to the column's own kind. Adds three tests mirroring the Python converter (apache/ossie#428). Co-authored-by: Isaac --- .../databricks/OssieToMetricView.java | 42 ++++++--- .../databricks/OssieConverterSuite.java | 90 +++++++++++++++++++ 2 files changed, 122 insertions(+), 10 deletions(-) diff --git a/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java b/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java index f7626848..90c0f677 100644 --- a/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java +++ b/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java @@ -1003,7 +1003,7 @@ private void drop(int index, boolean measure, int phase) { } Map column = (measure ? measures : dimensions).get(index); String name = (String) column.get("name"); - String reference = referencesDropped((String) column.get("expr"), name); + String reference = referencesDropped((String) column.get("expr"), name, measure); if (reference == null) { return; } @@ -1026,13 +1026,13 @@ private void propagate(String name, boolean measureReference, int phase, int sou Pattern pattern = referencePattern(name, measureReference); for (int index = 0; index < dimensions.size(); index++) { if (!isPending(index, false) - && matches(dimensions.get(index), name, measureReference, pattern)) { + && matches(dimensions.get(index), name, measureReference, false, pattern)) { schedule(index, false, phase, sourceIndex); } } for (int index = 0; index < measures.size(); index++) { if (!isPending(index, true) - && matches(measures.get(index), name, measureReference, pattern)) { + && matches(measures.get(index), name, measureReference, true, pattern)) { schedule(index, true, phase, sourceIndex); } } @@ -1047,10 +1047,23 @@ private boolean isPending(int index, boolean measure) { return removedDims.get(index) || dimsNow.get(index) || dimsNext.get(index); } + /** + * A column is a self-reference to a dropped name only within its own kind, compared + * case-insensitively (Databricks SQL identifiers are case-insensitive). A column that merely + * shares a dropped column's name across kinds (a measure vs a dropped dimension, or vice + * versa) is a different object, so it is still matched and cascade-dropped. Shared by both + * cascade paths -- the schedule path (matches) and the confirm path (referencesDropped) -- so + * the two guards cannot drift. + */ + private static boolean isSelfReference( + boolean columnIsMeasure, boolean droppedIsMeasure, String columnName, String droppedName) { + return columnIsMeasure == droppedIsMeasure && droppedName.equalsIgnoreCase(columnName); + } + private static boolean matches(Map column, String reference, - boolean measureReference, Pattern pattern) { + boolean measureReference, boolean columnIsMeasure, Pattern pattern) { String name = (String) column.get("name"); - if (!measureReference && reference.equals(name)) { + if (isSelfReference(columnIsMeasure, measureReference, name, reference)) { return false; } String expr = (String) column.get("expr"); @@ -1073,19 +1086,25 @@ private void schedule(int index, boolean measure, int phase, int sourceIndex) { } } - private String referencesDropped(String expr, String selfName) { + private String referencesDropped(String expr, String selfName, boolean selfIsMeasure) { // Databricks SQL identifiers are case-insensitive, so match a dropped name against the // expression case-insensitively: both the pre-filter here and the regex in referencePattern. + // The per-kind self-reference exemption is isSelfReference (shared with matches). String lowerExpr = expr.toLowerCase(Locale.ROOT); for (String name : droppedMeasures) { - if (name != null && lowerExpr.contains(name.toLowerCase(Locale.ROOT)) + if (name == null || isSelfReference(selfIsMeasure, true, selfName, name)) { + continue; + } + if (lowerExpr.contains(name.toLowerCase(Locale.ROOT)) && findOutsideLiterals(expr, referencePattern(name, true))) { return name; } } for (String name : droppedDims) { - if (name != null && !name.equals(selfName) - && lowerExpr.contains(name.toLowerCase(Locale.ROOT)) + if (name == null || isSelfReference(selfIsMeasure, false, selfName, name)) { + continue; + } + if (lowerExpr.contains(name.toLowerCase(Locale.ROOT)) && findOutsideLiterals(expr, referencePattern(name, false))) { return name; } @@ -1095,9 +1114,12 @@ && findOutsideLiterals(expr, referencePattern(name, false))) { private Pattern referencePattern(String name, boolean measure) { Map patterns = measure ? measurePatterns : dimensionPatterns; + // A dimension is referenced as a bare, unqualified token. The trailing (?!\s*\() excludes a + // same-spelled function/keyword call such as YEAR(...), which is a function, not a reference + // to a dropped `year` dimension. return patterns.computeIfAbsent(name, ignored -> Pattern.compile(measure ? "measure\\(\\s*" + Pattern.quote(name) + "\\s*\\)" - : "(? view = (Map) OssieConverter.parseYaml(result.yaml); + List measures = (List) view.get("measures"); + // YEAR(...) is a function call, not a reference to the dropped `year`, so the measure survives. + assertEquals(1, measures.size(), result.yaml); + assertEquals("orders_2026", ((Map) measures.get(0)).get("name")); + } + + @Test + @SuppressWarnings("unchecked") + public void cascadeDropKeepsASurvivorDifferingFromADroppedFieldOnlyInCase() { + // A dropped field and a surviving dimension whose names differ only in case can coexist. The + // survivor's bare self-reference (`region`) is its own name, not a reference to the dropped + // `REGION`, so the self-guard is case-folded and the survivor is kept. + String osi = + "version: \"0.2.0.dev0\"\n" + + "name: m\n" + + "datasets:\n" + + " - name: d\n" + + " source: cat.sch.t\n" + + " fields:\n" + + " - name: REGION\n" + + " expression:\n" + + " dialects: [{dialect: T_SQL, expression: REGION}]\n" + + " - name: region\n" + + " expression:\n" + + " dialects: [{dialect: DATABRICKS, expression: region}]\n"; + OssieConverter.Result result = OssieConverter.convertOssieToMetricView(osi, null); + Map view = (Map) OssieConverter.parseYaml(result.yaml); + List dims = (List) view.get("dimensions"); + assertEquals(1, dims.size(), result.yaml); + assertEquals("region", ((Map) dims.get(0)).get("name")); + } + + @Test + @SuppressWarnings("unchecked") + public void cascadeDropDropsAMeasureNamedLikeADroppedDimension() { + // A measure that shares a dropped dimension's name -- same name, SAME case -- is not a + // self-reference: the guard is scoped to the column's own kind, so the measure's reference to + // the dropped dimension is detected and the measure is cascade-dropped rather than emitted + // dangling. The same-case clash is what pins the kind-scoping: a case-only difference already + // cascade-drops without it (the pre-fix guard was case-sensitive), so this must match case. + String osi = + "version: \"0.2.0.dev0\"\n" + + "name: m\n" + + "datasets:\n" + + " - name: d\n" + + " source: cat.sch.t\n" + + " fields:\n" + + " - name: id\n" + + " expression:\n" + + " dialects: [{dialect: DATABRICKS, expression: id}]\n" + + " - name: region\n" + + " expression:\n" + + " dialects: [{dialect: T_SQL, expression: region}]\n" + + "metrics:\n" + + " - name: region\n" + + " expression:\n" + + " dialects: [{dialect: DATABRICKS, expression: SUM(region)}]\n"; + OssieConverter.Result result = OssieConverter.convertOssieToMetricView(osi, null); + Map view = (Map) OssieConverter.parseYaml(result.yaml); + assertFalse(view.containsKey("measures"), + "measure 'region' must cascade-drop, got: " + view.get("measures")); + assertTrue(result.notices.contains(cascadeNotice("measure", "region", "region")), + result.notices.toString()); + } + @Test public void stashPreservesABackslashBeforeAUnicodeEscape() { // The stash lowercases the hex of Jackson's \\uXXXX escapes. That rewrite must re-emit any From 6164f532953bc5a58225c09f8aa6c83861e62508 Mon Sep 17 00:00:00 2001 From: Haoran Li Date: Thu, 1 Oct 2026 21:21:01 +0000 Subject: [PATCH 2/2] fix(databricks): notice dropped datatype/extensions, clarify scalar join-columns error On export, a field or metric `datatype` and a relationship's foreign-vendor `custom_extensions` were dropped silently; both now emit a notice, matching the model/dataset/field levels and the "dropped with a notice" contract. Scalar `from_columns`/`to_columns` (not a list) now report "must be lists" rather than the misleading "are required". Updates the README mapping (model `description`, `datatype`) and drops stale `semantic_model`/"first semantic model" wording. Adds three tests. Co-authored-by: Isaac --- converters/databricks/java/README.md | 7 +- .../databricks/OssieToMetricView.java | 24 +++++-- .../databricks/OssieConverterSuite.java | 64 +++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/converters/databricks/java/README.md b/converters/databricks/java/README.md index f2983678..b77c9255 100644 --- a/converters/databricks/java/README.md +++ b/converters/databricks/java/README.md @@ -35,7 +35,8 @@ The directions are named from the Apache Ossie model's point of view, matching t On **export** (Apache Ossie -> Metric View), Apache Ossie features with no Metric View slot -- relationship `ai_context`, the non-`synonyms` members of a field/metric `ai_context` object, -`dimension.is_time`, foreign-vendor `custom_extensions` -- are **dropped with a notice**. An +a field/metric `datatype`, `dimension.is_time`, foreign-vendor `custom_extensions` -- are +**dropped with a notice**. An expression prefers the `DATABRICKS` dialect, then `ANSI_SQL`, then `OSSIE_SQL_2026` (Apache Ossie's portable, ANSI-SQL-compatible dialect); the other dialect alternatives are ignored (no notice) when a supported one is present, while a field or metric with no supported dialect is @@ -108,7 +109,7 @@ Each row maps in both directions; the **Notes** flag where a behavior is specifi | Apache Ossie | Metric View (v1.1) | Notes | |---|---|---| -| `semantic_model.description` | `comment` | Model-level description only. | +| model `description` | `comment` | Model-level description only. | | root dataset | `source` | The fact/grain. | | other `datasets` | nested `joins[]` | Export: the relationship graph is reassembled into the join tree; a dataset reached by two paths (a diamond) fans out into one aliased join per path. | | `relationship` `from_columns`/`to_columns` | join `on` (differing names) / `using` (shared names) | Decomposed into columns on import; rebuilt into `on`/`using` on export. A join `on` that is non-equi, function-wrapped, or carries an extra filter has no equi-join relationship form, so it is rejected on import (matching the Python converter) rather than emitting a relationship with empty column lists. | @@ -134,7 +135,7 @@ an input breaks one of these: - an Apache Ossie -> Metric View conversion has more than 200 distinct datasets or expands to more than 200 join nodes, or a Metric View -> Apache Ossie conversion produces more than 200 datasets (the same bound applies both ways, so anything that converts one way can convert back); -- on the first semantic model, a consumed `custom_extensions` value is not a list of mappings, or +- a consumed `custom_extensions` value is not a list of mappings, or has non-empty `DATABRICKS` `data` that is not a string containing one strict JSON object. Duplicate keys, trailing tokens, YAML syntax, and non-object JSON roots are rejected; missing, null, or empty `data` is treated as an empty object. diff --git a/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java b/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java index 90c0f677..c8b130f7 100644 --- a/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java +++ b/converters/databricks/java/src/main/java/org/apache/ossie/converter/databricks/OssieToMetricView.java @@ -358,6 +358,11 @@ private static Object[] buildJoinTree( throw new ConversionException("Model '" + modelName + "': relationship '" + str(get(rel, "name")) + "' references an unknown dataset"); } + for (String key : new String[] {"from_columns", "to_columns"}) { + if (rel.get(key) != null && !(rel.get(key) instanceof List)) { + throw new ConversionException(scope + ": from_columns and to_columns must be lists"); + } + } } List> relationships = new ArrayList<>(); for (Map rel : relationships0) { @@ -1170,10 +1175,13 @@ private static void warnDroppedModel(Map model, Notices notices) } for (Object relObj : asList(get(model, "relationships"))) { Map rel = asMap(relObj); + String rn = rel.containsKey("name") ? str(get(rel, "name")) : ""; if (truthy(get(rel, "ai_context"))) { - String rn = rel.containsKey("name") ? str(get(rel, "name")) : ""; notices.warn("relationship '" + rn + "'", "relationship ai_context dropped"); } + if (!foreignVendorExtensions(rel).isEmpty()) { + notices.warn("relationship '" + rn + "'", "foreign-vendor custom_extensions dropped"); + } } } @@ -1186,16 +1194,20 @@ private static void warnDroppedField(Map field, String scope, No } /** - * Notices shared by a field and a metric: the members of an `ai_context` OBJECT that have no - * Metric View slot, and foreign-vendor extensions. + * Notices shared by a field and a metric: `datatype`, the members of an `ai_context` OBJECT that + * have no Metric View slot, and foreign-vendor extensions. * *

`ai_context` is `string | object` in the Apache Ossie schema. The string form maps to the * column comment (mergeDescription) and the object's `synonyms` maps to the column's synonyms, - * but every other object member -- `instructions`, `examples` -- has nowhere to go. Naming them - * keeps the "dropped with a notice" contract that the dataset- and model-level checks in - * warnDroppedModel already honour. + * but every other object member -- `instructions`, `examples` -- has nowhere to go. `datatype` (a + * logical type on a field or metric) likewise has no Metric View slot, where a column's type is + * inferred from its expression. Naming them keeps the "dropped with a notice" contract that the + * dataset- and model-level checks in warnDroppedModel already honour. */ private static void warnDroppedColumn(Map column, String scope, Notices notices) { + if (get(column, "datatype") != null) { + notices.warn(scope, "datatype has no Metric View counterpart; dropped"); + } Object aiContext = get(column, "ai_context"); if (aiContext instanceof Map) { List dropped = new ArrayList<>(); diff --git a/converters/databricks/java/src/test/java/org/apache/ossie/converter/databricks/OssieConverterSuite.java b/converters/databricks/java/src/test/java/org/apache/ossie/converter/databricks/OssieConverterSuite.java index 6d702af9..58b5cc7d 100644 --- a/converters/databricks/java/src/test/java/org/apache/ossie/converter/databricks/OssieConverterSuite.java +++ b/converters/databricks/java/src/test/java/org/apache/ossie/converter/databricks/OssieConverterSuite.java @@ -498,6 +498,70 @@ public void foreignVendorExtensionDroppedWithNotice() { assertTrue(r.notices.stream().anyMatch(m -> m.contains("foreign-vendor custom_extensions dropped"))); } + @Test + public void datatypeDroppedWithNotice() { + // `datatype` (a logical type on a field or metric) has no Metric View slot, where a column's + // type is inferred from its expression, so it is dropped with a notice on both. + String osi = + "version: 0.2.0.dev0\n" + + "name: m\n" + + "datasets:\n" + + "- name: f\n" + + " source: c.s.f\n" + + " fields:\n" + + " - {name: d, datatype: String, expression: " + + "{dialects: [{dialect: DATABRICKS, expression: d}]}}\n" + + "metrics:\n" + + "- {name: n, datatype: Integer, expression: " + + "{dialects: [{dialect: DATABRICKS, expression: COUNT(*)}]}}\n"; + OssieConverter.Result r = OssieConverter.convertOssieToMetricView(osi, null); + assertTrue(r.notices.contains("[field 'd'] datatype has no Metric View counterpart; dropped"), + r.notices.toString()); + assertTrue(r.notices.contains("[metric 'n'] datatype has no Metric View counterpart; dropped"), + r.notices.toString()); + } + + @Test + public void relationshipForeignVendorExtensionDroppedWithNotice() { + // A foreign-vendor custom_extensions on a relationship has no Metric View slot, so it is + // dropped with a notice, matching the model/dataset/field levels. + String osi = + "version: 0.2.0.dev0\n" + + "name: m\n" + + "datasets:\n" + + "- name: o\n" + + " source: c.s.o\n" + + " fields:\n" + + " - {name: a, expression: {dialects: [{dialect: DATABRICKS, expression: a}]}}\n" + + "- {name: c, source: c.s.c, primary_key: [k]}\n" + + "relationships:\n" + + "- {name: oc, from: o, to: c, from_columns: [k], to_columns: [k], " + + "custom_extensions: [{vendor_name: SNOWFLAKE, data: '{}'}]}\n" + + "metrics:\n" + + "- {name: n, expression: {dialects: [{dialect: DATABRICKS, expression: COUNT(*)}]}}\n"; + OssieConverter.Result r = OssieConverter.convertOssieToMetricView(osi, null); + assertTrue(r.notices.contains("[relationship 'oc'] foreign-vendor custom_extensions dropped"), + r.notices.toString()); + } + + @Test + public void scalarJoinColumnsRejectedAsMustBeLists() { + // from_columns/to_columns given as a scalar (not a list) raise a clear "must be lists" error + // rather than the misleading "are required" (empty-list) or a character-count length error. + String osi = + "version: 0.2.0.dev0\n" + + "name: m\n" + + "datasets:\n" + + "- {name: a, source: c.s.a, fields: " + + "[{name: x, expression: {dialects: [{dialect: DATABRICKS, expression: x}]}}]}\n" + + "- {name: b, source: c.s.b}\n" + + "relationships:\n" + + "- {name: ab, from: a, to: b, from_columns: cid, to_columns: id}\n"; + OssieConverter.ConversionException e = assertThrows(OssieConverter.ConversionException.class, + () -> OssieConverter.convertOssieToMetricView(osi, null)); + assertTrue(e.getMessage().contains("must be lists"), e.getMessage()); + } + @Test public void emptyExplicitSourceFallsBackLikeAbsent() { // An empty --source (e.g. an unset shell variable) must be treated as absent and fall back to