Skip to content

fix(databricks): match cascade-drop references case-insensitively #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
christianeu-db wants to merge 2 commits into
base: main
Choose a base branch
from
+94 −10
Open

fix(databricks): match cascade-drop references case-insensitively #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
502d736
fix(databricks): match cascade-drop references case-insensitively
christianeu-db Sep 18, 2026
0103333
fix(databricks): exclude function calls from cascade-drop match
christianeu-db Sep 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions converters/databricks/src/ossie_databricks/ossie_to_metric_view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,20 +587,32 @@ def _references_dropped(expr, self_name, dropped_dims, dropped_measures):
Measures are only referenceable via `measure(<name>)` (exact). Dimensions are
referenced by their bare, *unqualified* name: a name that is part of a qualified
path (`alias.name` or `name.col`) is ignored, so a join alias or joined column
that merely shares a dropped dimension's name is not over-dropped. The one
ambiguity the regex can't resolve without a SQL parser is a bare, unqualified
*source column* sharing a dropped dimension's name -- there it errs on dropping.
that merely shares a dropped dimension's name is not over-dropped. A bare token
immediately followed by `(` is a function/keyword call (e.g. `COUNT(...)`), never
a dimension reference, so it is excluded too.

Without a real SQL parser the bare-name match still can't tell an identifier from
a same-spelled keyword that is *not* a call (a type in `CAST(x AS DATE)`, a unit
in `EXTRACT(YEAR FROM d)` / `INTERVAL 1 DAY`, `CASE ... END`, `DISTINCT`) or from
text inside a string literal; a dropped dimension named like one of those errs on
dropping. This is the residual the paren-guard does not close.

Matching is case-insensitive, as Databricks SQL identifiers are case-insensitive;
the self-reference guard is case-folded to match, so a measure or dimension is
never dropped for referencing itself under a different case.
"""
for m in dropped_measures:
if re.search(r"measure\(\s*" + re.escape(m) + r"\s*\)", expr):
if m.lower() != self_name.lower() and re.search(
r"measure\(\s*" + re.escape(m) + r"\s*\)", expr, re.IGNORECASE):
return m
for d in dropped_dims:
# Match only a bare, unqualified token: the negative look-behind/ahead for a
# word char or `.` excludes both substrings of a larger identifier and
# qualified paths (`alias.name` / `name.col`), so a join alias or joined
# column sharing a dropped name is not falsely cascade-dropped.
if d != self_name and re.search(
r"(?<![\w.])" + re.escape(d) + r"(?![\w.])", expr):
# Match only a bare, unqualified token that is not a function call: the
# negative look-behind/ahead for a word char or `.` excludes substrings of a
# larger identifier and qualified paths (`alias.name` / `name.col`), and the
# trailing `(?!\s*\()` excludes `NAME(...)` calls, so a dropped dim named e.g.
# `count` does not falsely match a surviving `COUNT(...)`.
if d.lower() != self_name.lower() and re.search(
r"(?<![\w.])" + re.escape(d) + r"(?![\w.])(?!\s*\()", expr, re.IGNORECASE):

@jbonofre jbonofre Sep 20, 2026

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.

I believe this flag makes the converter silently drop valid dimensions.

Let's take this:

fields:
    - name: active # DATABRICKS
    - name: status # DATABRICKS: CASE WHEN active THEN 'a' ELSE 'b' END
    - name: end.    # T_SQL only -> dropped

I think end is dropped. The END of CASE ... END now matches the dropped end. The new (?!\s*\() guard doesn't help: these keywords aren't calls. Same failure for a dropped date against CAST(ts AS DATE) and a dropped year against EXTRACT(YEAR FROM ts).
I believe field names like a non-call SQL keyword (end, date, year, interval, rows, ...) are dropped.

Also, I believe the same happens for fields with name inside a string literal in another case (like STATUS).

I'm sorry I should have seen that during the first reivew.

The case insensitive matching is right here, but it only pays for itself once the matcher strips quoted literals and skips SQL reserved words first.

I suggest replacing the regex with a single tokenizing pass: strip literals, tokenize [A-Za-z]\w*, skip tokens adjacent to . or followed by (, filter reserved word set, lowercase into a set. It would address the dropped fields plus the measure( gaps (see my next comment on the same line 😄 ).

@jbonofre jbonofre Sep 20, 2026

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.

Oh also, considering this:

datasets:
  - name: d
    fields:
      - name: id      # DATABRICKS
      - name: Region  # T_SQL only -> dropped
metrics:
  - name: region      # DATABRICKS: SUM(Region)

I believe the case-folding the self-guard suppresses a genuine cascade-drop, re-introducing the dangling reference this PR is meant to fix.

'Region'.lower() == 'region', so the guard treats a reference to the dropped fiels as a self-reference and keeps the measure. Note this reaches the cascade check at all only because dropped field name never enter seen_dims, so the case-insensitive name collision that would normally reject this model never fires.

test_cascade_drop_does_not_drop_self_reference_differing_only_in_case does pass, but the case it pins (a survivor whose expression is its own bare name) is narrower than the guard it justified, the name guard also swallows references to a different, dropped column.

I think two things worth separating here:

  • The .lower() on the self-guard isn't part of the case-insensitive fix and should come off.
  • The guard compares against self_name for both loops, so a measure named like a dropped dimension is exempted from a check that has nothing to do with it (field region dropped + metric region = COUNT(DISTINCT region) emits a dangling ref). Passing a kind through from _cascade_drop so the self guard only applies within the matching dropped set would fix that.

return d
return None

Expand Down
72 changes: 72 additions & 0 deletions converters/databricks/tests/test_ossie_to_metric_view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,78 @@ def test_cascade_drop_downstream_dimension_reference():
assert dims == ["keep"] # region dropped; label cascade-dropped; keep survives


def test_cascade_drop_matches_dropped_name_case_insensitively():
"""Databricks SQL identifiers are case-insensitive, so a measure that references a
dropped field in a different case (COUNT(DISTINCT REGION_NAME) over a dropped
region_name) must cascade-drop rather than survive as a dangling reference."""
import yaml
ossie = yaml.safe_dump({
"version": exporter.OSSIE_VERSION,
"name": "m",
"datasets": [{"name": "d", "source": "c.s.t", "fields": [
{"name": "id", "expression": {"dialects": [{"dialect": "DATABRICKS", "expression": "id"}]}},
{"name": "region_name", "expression": {"dialects": [{"dialect": "T_SQL", "expression": "region_name"}]}}, # dropped: no DBX/ANSI
]}],
"metrics": [
# references the dropped region_name in upper case
{"name": "region_count", "expression": {"dialects": [
{"dialect": "DATABRICKS", "expression": "COUNT(DISTINCT REGION_NAME)"}]}},
],
})
out = parse(exporter.convert_ossie_to_metric_view(ossie))
measures = [m["name"] for m in out.get("measures", [])]
dims = [d["name"] for d in out.get("dimensions", [])]
assert measures == [] # region_count cascade-dropped despite the case mismatch
assert dims == ["id"] # the unrelated dimension survives


def test_cascade_drop_does_not_over_drop_function_named_like_dropped_field():
"""A dropped field whose name collides with a SQL function/keyword token in a
surviving expression must NOT cascade-drop it. `COUNT(...)` is a function call,
not a reference to a dropped `count` dimension. Because identifiers are matched
case-insensitively, the collision would otherwise fire on any case, so the match
must exclude function-call tokens (`NAME(...)`)."""
import yaml
ossie = yaml.safe_dump({
"version": exporter.OSSIE_VERSION,
"name": "m",
"datasets": [{"name": "d", "source": "c.s.t", "fields": [
{"name": "id", "expression": {"dialects": [{"dialect": "DATABRICKS", "expression": "id"}]}},
# dropped (no DBX/ANSI); its bare name collides with the COUNT() function
{"name": "count", "expression": {"dialects": [{"dialect": "T_SQL", "expression": "count"}]}},
]}],
"metrics": [
{"name": "total", "expression": {"dialects": [
{"dialect": "DATABRICKS", "expression": "COUNT(DISTINCT id)"}]}},
],
})
out = parse(exporter.convert_ossie_to_metric_view(ossie))
measures = [m["name"] for m in out.get("measures", [])]
# `total` is not cascade-dropped: COUNT(...) is a function call, not a `count` ref.
assert measures == ["total"]


def test_cascade_drop_does_not_drop_self_reference_differing_only_in_case():
"""A dropped field and a surviving dimension whose names differ only in case can
coexist (a dropped field is not deduped against survivors). The survivor's bare
self-reference must not be read as a reference to the dropped field, so the
self-guard is case-folded and the survivor is kept rather than cascade-dropped."""
import yaml
ossie = yaml.safe_dump({
"version": exporter.OSSIE_VERSION,
"name": "m",
"datasets": [{"name": "d", "source": "c.s.t", "fields": [
# dropped (no DBX/ANSI); name differs from the survivor only in case
{"name": "REGION", "expression": {"dialects": [{"dialect": "T_SQL", "expression": "REGION"}]}},
{"name": "region", "expression": {"dialects": [{"dialect": "DATABRICKS", "expression": "region"}]}},
]}],
})
out = parse(exporter.convert_ossie_to_metric_view(ossie))
dims = [d["name"] for d in out.get("dimensions", [])]
# `region` survives: its bare `region` is a self-reference, not a ref to dropped REGION
assert dims == ["region"]


def test_orientation_unverifiable_when_to_side_has_no_key_warns():
"""If the `from` columns are a declared key but the `to` side declares no key, the
from/to orientation can't be verified; the converter leaves it as-is (no reorient)
Expand Down
Loading