Skip to content

Commit 4e0e95c

Browse files
NQPhucNQPhuc
authored
Merge pull request #873 from holistics/fix-dbml-logic-union-intersect
DBX-6689 Fix wrong union in DiagramView
2 parents aece85b + 293c44d commit 4e0e95c

File tree

16 files changed

+2400
-78
lines changed

16 files changed

+2400
-78
lines changed

‎ai-contracts/dbml-union-intersect-diagram-view/01-problem-analysis.md‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
---
2+
ai_summary: "DiagramView wildcard (*) in one Trinity dim gets ignored when sibling dims have explicit values — filterNormalizedModel treats [] as 'no filter' instead of 'show all'"
3+
ai_warnings:
4+
- "Two changes needed: (1) dbml parser must always use union semantics and output concrete lists for wildcards, (2) syncDiagramView must not emit * when user has specific filters"
5+
ai_generated_at: "2026-04-15"
6+
mode: "refactor"
7+
attached_repos:
8+
- dbml
9+
- dbx-utils
10+
---
11+
12+
# Root Cause Analysis: DiagramView Wildcard Ignored When Sibling Dims Have Explicit Values
13+
14+
## Current Behavior
15+
16+
When a DiagramView block uses `*` (wildcard) in one Trinity dimension alongside explicit values in other dimensions, the wildcard dimension is silently ignored:
17+
18+
```dbml
19+
DiagramView "MyView" {
20+
Tables: users, orders
21+
TableGroups: *
22+
Schemas: sales
23+
}
24+
```
25+
26+
**Result**: Shows `users`, `orders`, and all tables in schema `sales` — but **TableGroups are completely ignored**. No defined table groups are shown.
27+
28+
## Expected Behavior
29+
30+
Each Trinity dimension should independently contribute to the visible set (union/OR semantics):
31+
- `Tables: users, orders` → show users, orders
32+
- `TableGroups: *` → show ALL defined table groups (and their member tables)
33+
- `Schemas: sales` → show ALL tables in schema `sales`
34+
- **Result**: union of all three — users, orders, all tables in schema sales, plus all defined table groups
35+
36+
## Root Cause
37+
38+
The bug spans two layers — the **dbml parser** and the **filterNormalizedModel** utility in dbx-utils.
39+
40+
### Layer 1: `filterNormalizedModel.ts` (dbx-utils)
41+
42+
The `hasTableGroupFilters` check (line 128):
43+
```typescript
44+
const hasTableGroupFilters = tableGroupFilters !== null && tableGroupFilters !== undefined && !isEmpty(tableGroupFilters);
45+
```
46+
47+
- `[]` (empty array) → `hasTableGroupFilters = false` (because `isEmpty([])` is `true`)
48+
- But `[]` is documented as "SHOW ALL" (line 103): "`[]` (empty array) = SHOW ALL items of that type"
49+
- The OR logic (line 275) only considers a dimension if `has*Filters` is `true`:
50+
```typescript
51+
const isTableIncluded = isTableInTableFilter || isTableInGroupFilter || isTableInSchemaFilter;
52+
```
53+
Since `hasTableGroupFilters = false`, `isTableInGroupFilter` is always `false`.
54+
55+
**Same issue applies to `hasTableFilters` and `hasSchemaFilters`** — empty arrays mean "show all" but are treated as "inactive filter".
56+
57+
### Layer 2: `expandDiagramViewWildcards` (dbml)
58+
59+
When `TableGroups: *` is combined with other Trinity dims:
60+
1. `DiagramViewInterpreter` sets `tableGroups = []` and tracks it as a wildcard in `diagramViewWildcards`
61+
2. `expandDiagramViewWildcards` checks (line 84): `otherTrinitySet = explicitlySet.has('tables') || explicitlySet.has('schemas')`
62+
3. When other dims ARE set, it **refuses to expand** the wildcard — keeping `tableGroups = []`
63+
4. This `[]` then hits `filterNormalizedModel` where it's treated as "no filter" → table groups ignored
64+
65+
### Layer 3: `syncDiagramView.ts` — generates `*` unnecessarily
66+
67+
`generateDiagramViewBlock` (line 91-93):
68+
```typescript
69+
} else if (visibleEntities.tables.length === 0) {
70+
lines.push(' Tables { * }');
71+
}
72+
```
73+
74+
When the frontend passes `tables: []` (meaning "user didn't filter tables"), `syncDiagramView` emits `Tables { * }`. This creates a block with `*` even when the user only intended to filter by schemas or table groups.
75+
76+
### The Full Bug Chain
77+
78+
1. User creates a view with `Tables: [users, orders]`, `Schemas: [sales]`, `TableGroups: []` (untouched)
79+
2. `syncDiagramView` generates: `Tables { users, orders }` + `TableGroups { * }` + `Schemas { sales }` (because `tableGroups: []` → emits `*`)
80+
3. Parser sets `tableGroups = []`, tracks as wildcard, but `expandDiagramViewWildcards` won't expand it (other Trinity dims are set)
81+
4. `filterNormalizedModel` receives `tableGroups = []`, `hasTableGroupFilters = false` → table groups ignored
82+
5. Wrong result: no table groups shown
83+
84+
## Affected Code
85+
86+
| File | Repo | Issue |
87+
|------|------|-------|
88+
| `packages/dbml-parse/src/core/interpreter/elementInterpreter/diagramView.ts` | dbml | Wildcard `[]` not expanded to concrete list when sibling dims are set |
89+
| `packages/dbml-parse/src/core/interpreter/interpreter.ts:expandDiagramViewWildcards` | dbml | Only expands tableGroups wildcard when it's the ONLY Trinity dim |
90+
| `packages/dbml-parse/src/compiler/queries/transform/syncDiagramView.ts:generateDiagramViewBlock` | dbml | Emits `*` when user didn't filter a dimension (empty array) |
91+
| `src/filterNormalizedModel.ts` | dbx-utils | `has*Filters` treats `[]` as "no filter" instead of "show all" |
92+
93+
## Two Required Changes
94+
95+
### Change 1: Parser always uses union rules (dbml)
96+
97+
The parser should expand `*` wildcards to concrete lists **unconditionally** — regardless of whether sibling Trinity dims are set. This means:
98+
- `Tables: *` → expand to all table names (from `env.tables`)
99+
- `TableGroups: *` → expand to all table group names (from `env.tableGroups`)
100+
- `Schemas: *` → expand to all schema names (from `env.schemas`)
101+
102+
After expansion, `filterNormalizedModel` receives concrete non-empty arrays and the `has*Filters` checks work correctly.
103+
104+
**Key concern**: The output must remain compatible with `dbdiagram-frontend`. Currently the frontend uses `getFilterConfig()` which treats `[]` as "show all". After the change, `*` will be expanded to concrete lists, so the frontend will receive specific items instead of `[]`. Need to verify this doesn't break:
105+
- View switching (loading view filter from parsed data)
106+
- Default view filter logic
107+
- Table rename propagation across views
108+
109+
### Change 2: `syncDiagramView` should not emit `*` (dbml)
110+
111+
When generating DBML from UI operations, `generateDiagramViewBlock` should **omit** dimensions the user didn't filter (where the array is empty = "show all"). This prevents the parser from ever seeing `*` for cases where the user only filtered some dimensions.
112+
113+
```typescript
114+
// Before: empty array → emits "Tables { * }"
115+
// After: empty array → omits the Tables block entirely
116+
if (visibleEntities?.tables !== undefined) {
117+
if (visibleEntities.tables === null) {
118+
// Hide all - don't add block
119+
} else if (visibleEntities.tables.length === 0) {
120+
// Show all - omit block (was: "Tables { * }")
121+
} else {
122+
// Specific items
123+
}
124+
}
125+
```
126+
127+
This is safe because the "Trinity omit rule" already handles omitted dims:
128+
- If any Trinity dim IS set → omitted dims get `[]` (show all)
129+
- If NO Trinity dim is set → everything stays `null` (handled by filterNormalizedModel)
130+
131+
## Reproduction Steps
132+
133+
1. Create a diagram with multiple tables, table groups, and schemas
134+
2. Write DBML:
135+
```dbml
136+
DiagramView "Test" {
137+
Tables { users, orders }
138+
TableGroups { * }
139+
Schemas { sales }
140+
}
141+
```
142+
3. Parse and render the diagram
143+
4. Observe: table groups are not shown
144+
5. Expected: all defined table groups should be visible
145+
146+
## Open Questions
147+
148+
- [ ] Should `expandDiagramViewWildcards` be expanded to handle ALL Trinity dims (tables, schemas, notes) or just tableGroups?
149+
- [ ] Does the dbx-utils `filterNormalizedModel` need changes, or is the dbml-only fix (always expanding wildcards to concrete lists) sufficient?
150+
- [ ] Are there callers of `syncDiagramView` that depend on `*` being emitted for "show all" dimensions?

‎ai-contracts/dbml-union-intersect-diagram-view/02-solutions.md‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
---
2+
ai_summary: "Fix DiagramView union semantics: always expand TableGroups wildcard to concrete names, rewrite syncDiagramView generation to follow 6 rules from filter-dbml-examples.md"
3+
ai_warnings:
4+
- "Only dbml changes needed — no dbx-utils changes"
5+
ai_generated_at: "2026-04-15"
6+
attached_repos:
7+
- dbml
8+
---
9+
10+
# Solutions: DiagramView Union Semantics Fix
11+
12+
## Overview
13+
14+
```mermaid
15+
flowchart LR
16+
subgraph "syncDiagramView (generation)"
17+
A["UI FilterConfig"] -->|"6 rules"| B["DBML: correct representation"]
18+
end
19+
20+
subgraph "Parser (interpretation)"
21+
B -->|"Trinity omit rule"| C["omitted dims → [] (show all)"]
22+
B -->|"wildcard expansion"| D["TableGroups: * → concrete names"]
23+
B -->|"body-level * → all []"| E["no expansion needed"]
24+
end
25+
26+
subgraph "filterNormalizedModel (rendering)"
27+
C --> F["[] treated as show-all"]
28+
D --> G["concrete names → name lookup works"]
29+
F --> H["Union: all matching tables"]
30+
G --> H
31+
end
32+
```
33+
34+
Two changes, both in dbml, working together:
35+
36+
1. **syncDiagramView** — follows 6 generation rules to produce correct DBML. Handles legacy null cases and normal cases via omit-empty and union rules. Never emits unnecessary `*` for dimensions the user didn't filter.
37+
38+
2. **expandDiagramViewWildcards** — for human-written DBML with `TableGroups: *`, always expands to concrete group names regardless of sibling dims. Body-level `{ * }` is unaffected.
39+
40+
3. **No dbx-utils changes needed** — expanding TableGroups `*` to concrete names gives `filterNormalizedModel` non-empty arrays, so the existing `hasTableGroupFilters` check correctly returns `true`.
41+
42+
### Round-trip Truth Tables
43+
44+
> Shorthand: `(T, TG, S, N)` = FilterConfig. `n` = null, `[]` = empty array, `[x]` = has items.
45+
> Delta column shows what changed in round-trip (blank = 1:1).
46+
47+
#### Table 1: FilterConfig → DBML → FilterConfig (Generation Stability)
48+
49+
Tests: does `generate(filterConfig)` produce DBML that re-parses back to the same FilterConfig?
50+
51+
| Case | Input FC | → DBML | → FC (re-parse) | Delta |
52+
|------|----------|--------|-----------------|-------|
53+
| **A1** | `n, n, n, n` | `{ }` | `n, n, n, n` | — |
54+
| **A2** | `[T], n, [], []` | `Tables {T}` | `[T], [], [], n` | TG:`n→[]`, N:`[]→n` |
55+
| **A3** | `[T], n, [S], []` | `Tables {T} Schemas {S}` | `[T], [], [S], n` | TG:`n→[]`, N:`[]→n` |
56+
| **A5** | `n, [], [], []` | `Notes { * }` | `n, n, n, []` | TG:`[]→n`, S:`[]→n` |
57+
| **A6** | `[], [], n, []` | `Notes { * }` | `n, n, n, []` | T:`[]→n`, TG:`[]→n` |
58+
| **A7** | `n, [], n, []` | `Notes { * }` | `n, n, n, []` | TG:`[]→n` |
59+
| **A8** | `n, [TG], [S], []` | `TableGroups {TG} Schemas {S}` | `[], [TG], [S], n` | T:`n→[]`, N:`[]→n` |
60+
| **A9** | `[T], [TG], n, []` | `Tables {T} TableGroups {TG}` | `[T], [TG], [], n` | S:`n→[]`, N:`[]→n` |
61+
| **A10** | `n, n, n, []` | `Notes { * }` | `n, n, n, []` | — |
62+
| **B1** | `[], [], [], []` | `{ * }` | `[], [], [], []` | — |
63+
| **B2** | `[T], [], [], []` | `Tables {T}` | `[T], [], [], n` | N:`[]→n` |
64+
| **B3** | `[], [TG], [], []` | `TableGroups {TG}` | `[], [TG], [], n` | N:`[]→n` |
65+
| **B4** | `[], [], [S], []` | `Schemas {S}` | `[], [], [S], n` | N:`[]→n` |
66+
| **B5** | `[T], [], [S], []` | `Tables {T} Schemas {S}` | `[T], [], [S], n` | N:`[]→n` |
67+
| **B6** | `[T], [TG], [], []` | `Tables {T} TableGroups {TG}` | `[T], [TG], [], n` | N:`[]→n` |
68+
| **B7** | `[], [TG], [S], []` | `TableGroups {TG} Schemas {S}` | `[], [TG], [S], n` | N:`[]→n` |
69+
| **B8** | `[T], [TG], [S], []` | `Tables {T} TableGroups {TG} Schemas {S}` | `[T], [TG], [S], n` | N:`[]→n` |
70+
| **C1** | `[], [], [], [N]` | `Tables { * } Notes {N}` | `[], [], [], [N]` | — |
71+
| **C2** | `[T], [], [], [N]` | `Tables {T} Notes {N}` | `[T], [], [], [N]` | — |
72+
73+
**Non-1:1 patterns:**
74+
75+
| Pattern | Cause | Why acceptable |
76+
|---------|-------|----------------|
77+
| N:`[]→n` | When no Notes block emitted, parser leaves stickyNotes as `null` | Both `[]` and `null` mean "don't filter notes" in practice; filterNormalizedModel handles both |
78+
| TG:`n→[]` | Trinity omit: when other Trinity dims are set, omitted tableGroups promoted to `[]` | `null` tableGroups is legacy; after round-trip it becomes `[]` (show all) — functionally equivalent |
79+
| S:`n→[]` / T:`n→[]` | Same Trinity omit rule | Same reason — `null` dims only exist in legacy data |
80+
| T:`[]→n` / TG:`[]→n` / S:`[]→n` | `Notes { * }` generation: only Notes block emitted, no Trinity set → no Trinity omit | Legacy "show only notes" case; empty arrays → null is functionally equivalent |
81+
82+
#### Table 2: DBML → FilterConfig → DBML (Parse Stability)
83+
84+
Tests: does parsing DBML produce a FilterConfig that generates back to the same DBML?
85+
86+
| Case | Input DBML | → FC | → DBML (re-generate) | Delta |
87+
|------|-----------|------|---------------------|-------|
88+
| **D1** | `Tables { users, orders }` | `[T], [], [], n` | `Tables { users, orders }` | — |
89+
| **D2** | `TableGroups { Inv, Rep }` | `[], [TG], [], n` | `TableGroups { Inv, Rep }` | — |
90+
| **D3** | `Schemas { sales }` | `[], [], [S], n` | `Schemas { sales }` | — |
91+
| **D4** | `Tables { users } Schemas { sales }` | `[T], [], [S], n` | `Tables { users } Schemas { sales }` | — |
92+
| **D5** | `Tables { users } TableGroups { Inv }` | `[T], [TG], [], n` | `Tables { users } TableGroups { Inv }` | — |
93+
| **D6** | `TableGroups { Inv } Schemas { sales }` | `[], [TG], [S], n` | `TableGroups { Inv } Schemas { sales }` | — |
94+
| **D7** | All three have items | `[T], [TG], [S], n` | All three blocks | — |
95+
| **E1** | `Tables{T} TableGroups{*} Schemas{S}` | `[T], [TG], [S], n` | `Tables{T} TableGroups{TG} Schemas{S}` | `*` expanded to concrete names |
96+
| **E2** | `Tables { * }` | `[], [], [], n` | `{ * }` | `Tables{*}` → body `{*}` (stickyNotes null) |
97+
| **E3** | `Schemas { * }` | `[], [], [], n` | `{ * }` | `Schemas{*}` → body `{*}` |
98+
| **E4** | `TableGroups { * }` | `[], [TG], [], n` | `TableGroups {TG}` | `*` expanded to concrete names |
99+
| **F1** | `{ * }` | `[], [], [], []` | `{ * }` | — |
100+
| **F2** | `Tables{*} Notes{Note1}` | `[], [], [], [N]` | `Tables{*} Notes{Note1}` | — |
101+
| **G1** | `{ }` (empty body) | `n, n, n, n` | `{ }` | — |
102+
| **G2** | `Tables { }` (empty sub-block) | `n, n, n, n` | `{ }` | `Tables{}` → `{ }` (empty sub-block = all null) |
103+
| **H1** | `Notes { Note1, Note2 }` | `n, n, n, [N]` | `Notes { Note1, Note2 }` | — |
104+
105+
**Non-1:1 patterns:**
106+
107+
| Pattern | Cause | Why acceptable |
108+
|---------|-------|----------------|
109+
| `TableGroups{*}` → concrete names | Wildcard expanded to concrete group names | Expanded form is functionally identical; needed for union semantics |
110+
| `Tables{*}` → body `{*}` | When only `Tables{*}` (no other dims), FC is `[],[],[],n` → all Trinity empty → body-level `{*}` | Semantically equivalent; body `{*}` = show all |
111+
| `Tables{}` → `{ }` | Empty sub-block = all null → generates empty body | Correct — empty block is meaningless, same as no block |
112+
113+
---
114+
115+
## Sub-Problems
116+
117+
### Sub-Problem 1: `expandDiagramViewWildcards` — wildcard expansion rules
118+
119+
```mermaid
120+
flowchart TD
121+
A["Wildcard parsed"] --> B["expandDiagramViewWildcards runs"]
122+
B --> C{"wildcards.has('tables') OR wildcards.has('schemas')?"}
123+
C -->|yes| D["All Trinity dims → [] (show all)"]
124+
D --> E["Union covers everything — specific items overridden"]
125+
C -->|no| F{"wildcards.has('tableGroups')?"}
126+
F -->|yes| G["Expand to concrete group names"]
127+
G --> H["filterNormalizedModel receives concrete list"]
128+
H --> I["Union: tables + group members"]
129+
F -->|no| J["No expansion needed"]
130+
```
131+
132+
**Two wildcard rules:**
133+
134+
1. **Tables `*` or Schemas `*`** → all Trinity dims become `[]` (show all). Union with "show all tables" or "show all schemas" = show everything. Specific items in other dims are overridden.
135+
136+
2. **TableGroups `*`** → expand to concrete group names. Does NOT collapse other dims because not all tables belong to groups — specific items in Tables/Schemas are still meaningful.
137+
138+
**Why the difference:**
139+
- `Tables: *` = show all tables → union with anything = everything
140+
- `Schemas: *` = show all schemas → union with anything = everything
141+
- `TableGroups: *` needs concrete names because some tables DON'T belong to any group. Specific tables/schemas alongside group wildcard still filter meaningfully.
142+
143+
### Sub-Problem 2: `syncDiagramView` — rewrite generation rules
144+
145+
```mermaid
146+
flowchart TD
147+
A["FilterConfig from UI"] --> B{"All null?"}
148+
B -->|yes| C["{ }"]
149+
B -->|no| D{"Any Trinity null?"}
150+
D -->|yes, Trinity has items| E["Union: emit only items dims"]
151+
D -->|yes, Trinity empty, notes has items| F["Notes { items }"]
152+
D -->|yes, Trinity empty, notes empty| G["Notes { * }"]
153+
D -->|no| H{"All Trinity []?"}
154+
H -->|yes, notes empty| I["{ * }"]
155+
H -->|yes, notes has items| J["Tables { * } Notes { items }"]
156+
H -->|no| K["Emit only dims with items"]
157+
```
158+
159+
**6 generation rules:**
160+
161+
1. **`tableGroups: null`** → frontend backfills standalone tables into tables array; emit tables as-is, omit tableGroups
162+
2. **`tables: null` or `schemas: null` + rest empty** → `Notes { * }`
163+
3. **`tables: null` or `schemas: null` + rest has items** → union rule, omit null dim
164+
4. **All Trinity `[]`** → body-level `{ * }` (or `Tables { * } + Notes` if notes)
165+
5. **Some items + rest `[]`** → emit only items dims
166+
6. **All items** → emit all
167+
168+
**Safety guarantee:** Trinity omit rule ensures omitted dims → `[]` on re-parse, so omitting `[]` dims is always safe.

0 commit comments

Comments
 (0)