Skip to content

Commit 1e33c8b

Browse files
akoclaude
andcommitted
fix: DROP ENUMERATION errors on ambiguous unqualified name (#391)
The loop previously dropped the first enumeration found with a matching name when no module was specified, silently ignoring other modules that had an enumeration with the same name. Rewritten to collect all matches first: - 0 matches → not found - 1 match → delete (qualified or unambiguous unqualified) - 2+ matches → validation error listing all matching qualified names, asking the user to qualify the name Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e678735 commit 1e33c8b

2 files changed

Lines changed: 89 additions & 11 deletions

File tree

mdl/executor/cmd_enumerations.go

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -127,21 +127,42 @@ func execDropEnumeration(ctx *ExecContext, s *ast.DropEnumerationStmt) error {
127127
return mdlerrors.NewBackend("list enumerations", err)
128128
}
129129

130+
// Collect all enumerations matching the name (and optional module).
131+
type match struct {
132+
enum *model.Enumeration
133+
module *model.Module
134+
}
135+
var matches []match
130136
for _, enum := range enums {
131-
if enum.Name == s.Name.Name {
132-
// Check module matches
133-
module, err := findModuleByID(ctx, enum.ContainerID)
134-
if err == nil && (s.Name.Module == "" || module.Name == s.Name.Module) {
135-
if err := ctx.Backend.DeleteEnumeration(enum.ID); err != nil {
136-
return mdlerrors.NewBackend("delete enumeration", err)
137-
}
138-
fmt.Fprintf(ctx.Output, "Dropped enumeration: %s\n", s.Name)
139-
return nil
140-
}
137+
if enum.Name != s.Name.Name {
138+
continue
139+
}
140+
module, err := findModuleByID(ctx, enum.ContainerID)
141+
if err != nil {
142+
continue
143+
}
144+
if s.Name.Module == "" || module.Name == s.Name.Module {
145+
matches = append(matches, match{enum, module})
141146
}
142147
}
143148

144-
return mdlerrors.NewNotFound("enumeration", s.Name.String())
149+
switch len(matches) {
150+
case 0:
151+
return mdlerrors.NewNotFound("enumeration", s.Name.String())
152+
case 1:
153+
if err := ctx.Backend.DeleteEnumeration(matches[0].enum.ID); err != nil {
154+
return mdlerrors.NewBackend("delete enumeration", err)
155+
}
156+
fmt.Fprintf(ctx.Output, "Dropped enumeration: %s.%s\n", matches[0].module.Name, s.Name.Name)
157+
return nil
158+
default:
159+
var names []string
160+
for _, m := range matches {
161+
names = append(names, m.module.Name+"."+s.Name.Name)
162+
}
163+
return mdlerrors.NewValidationf("ambiguous enumeration name %q — found in: %s; use a fully-qualified name",
164+
s.Name.Name, strings.Join(names, ", "))
165+
}
145166
}
146167

147168
// listEnumerations handles SHOW ENUMERATIONS command.

mdl/executor/cmd_enumerations_mock_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,5 +98,62 @@ func TestDescribeEnumeration_Mock_NotFound(t *testing.T) {
9898
assertError(t, err)
9999
}
100100

101+
// Issue #391 — DROP ENUMERATION with an unqualified name must error when
102+
// the name matches enumerations in multiple modules, not silently drop one.
103+
func TestDropEnumeration_AmbiguousUnqualified_Issue391(t *testing.T) {
104+
mod1 := mkModule("Mod1")
105+
mod2 := mkModule("Mod2")
106+
e1 := mkEnumeration(mod1.ID, "Status", "Active", "Inactive")
107+
e2 := mkEnumeration(mod2.ID, "Status", "Open", "Closed")
108+
109+
h := mkHierarchy(mod1, mod2)
110+
withContainer(h, e1.ContainerID, mod1.ID)
111+
withContainer(h, e2.ContainerID, mod2.ID)
112+
113+
mb := &mock.MockBackend{
114+
IsConnectedFunc: func() bool { return true },
115+
ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod1, mod2}, nil },
116+
ListEnumerationsFunc: func() ([]*model.Enumeration, error) { return []*model.Enumeration{e1, e2}, nil },
117+
}
118+
119+
ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
120+
err := execDropEnumeration(ctx, &ast.DropEnumerationStmt{
121+
Name: ast.QualifiedName{Name: "Status"}, // unqualified
122+
})
123+
assertError(t, err)
124+
assertContainsStr(t, err.Error(), "ambiguous")
125+
}
126+
127+
func TestDropEnumeration_Qualified_Success(t *testing.T) {
128+
mod1 := mkModule("Mod1")
129+
mod2 := mkModule("Mod2")
130+
e1 := mkEnumeration(mod1.ID, "Status", "Active")
131+
e2 := mkEnumeration(mod2.ID, "Status", "Open")
132+
deleted := ""
133+
134+
h := mkHierarchy(mod1, mod2)
135+
withContainer(h, e1.ContainerID, mod1.ID)
136+
withContainer(h, e2.ContainerID, mod2.ID)
137+
138+
mb := &mock.MockBackend{
139+
IsConnectedFunc: func() bool { return true },
140+
ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod1, mod2}, nil },
141+
ListEnumerationsFunc: func() ([]*model.Enumeration, error) { return []*model.Enumeration{e1, e2}, nil },
142+
DeleteEnumerationFunc: func(id model.ID) error {
143+
deleted = string(id)
144+
return nil
145+
},
146+
}
147+
148+
ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
149+
err := execDropEnumeration(ctx, &ast.DropEnumerationStmt{
150+
Name: ast.QualifiedName{Module: "Mod1", Name: "Status"},
151+
})
152+
assertNoError(t, err)
153+
if deleted != string(e1.ID) {
154+
t.Errorf("expected Mod1.Status (id=%s) to be deleted, got %s", e1.ID, deleted)
155+
}
156+
}
157+
101158
// Backend error: cmd_error_mock_test.go (TestShowEnumerations_Mock_BackendError)
102159
// JSON: cmd_json_mock_test.go (TestShowEnumerations_Mock_JSON)

0 commit comments

Comments
 (0)