Skip to content

Commit 85b4e12

Browse files
macneale4claude
andcommitted
enforce foreign keys when reverting through dolt_workspace_TABLE (#11112)
DELETE on a dolt_workspace_* row reverts the pending change to the underlying table, but the bespoke WriteSession used for that revert bypassed FK enforcement. Reverting an added parent row could silently orphan children, and a subsequent dolt_commit would persist the FK violation -- which dolt_verify_constraints() then missed because its default check is the diff from HEAD, and the orphans were already in HEAD. Resolve the underlying table through the session catalog and wrap it in a plan.ForeignKeyEditor (via the newly exported analyzer.BuildForeignKeyEditor) so the workspace revert flows through the same FK machinery as ordinary DML: RESTRICT rejects the revert, CASCADE/SET NULL/SET DEFAULT act on children. The bespoke WriteSession is dropped from the DELETE path; UPDATE (staged flag toggle) is unchanged. Adds two regression tests covering RESTRICT and CASCADE on workspace DELETE of a parent row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 75273d6 commit 85b4e12

4 files changed

Lines changed: 148 additions & 16 deletions

File tree

go/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ require (
6464
github.com/dolthub/dolt-mcp v0.3.4
6565
github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69
6666
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
67-
github.com/dolthub/go-mysql-server v0.20.1-0.20260527162314-8764587064e4
67+
github.com/dolthub/go-mysql-server v0.20.1-0.20260528203524-59a6a564f52a
6868
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63
6969
github.com/edsrzf/mmap-go v1.2.0
7070
github.com/esote/minmaxheap v1.0.0

go/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ github.com/dolthub/fslock v0.0.4 h1:p0FcUPu81bO53349qPT1im7XyGu3CwZDsB0PEVdFcaE=
214214
github.com/dolthub/fslock v0.0.4/go.mod h1:895XJNoMTlL5N1TjOJbd9Pnpjtv1DW/Ddw9qShcAYsM=
215215
github.com/dolthub/go-icu-regex v0.0.0-20260412212219-49724d547866 h1:U6gSf5I0e6h6GP1/5Sa7D2lWW1CWfcVPtY5wkyHq6jY=
216216
github.com/dolthub/go-icu-regex v0.0.0-20260412212219-49724d547866/go.mod h1:F3cnm+vMRK1HaU6+rNqQrOCyR03HHhR1GWG2gnPOqaE=
217-
github.com/dolthub/go-mysql-server v0.20.1-0.20260527162314-8764587064e4 h1:ioyMzpxVr7PDwgwMQxZ0f66+2LUC0cECSSj94pDaFfg=
218-
github.com/dolthub/go-mysql-server v0.20.1-0.20260527162314-8764587064e4/go.mod h1:482+GNJMKMZ6F0iq1wiAJ0Gg9ha4Y+z3LlBmwZZIanY=
217+
github.com/dolthub/go-mysql-server v0.20.1-0.20260528203524-59a6a564f52a h1:Mgl9aWp9v2eQYtMPA1hjfcwHgb3mKDHpLgyygi6CwsM=
218+
github.com/dolthub/go-mysql-server v0.20.1-0.20260528203524-59a6a564f52a/go.mod h1:482+GNJMKMZ6F0iq1wiAJ0Gg9ha4Y+z3LlBmwZZIanY=
219219
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
220220
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
221221
github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037 h1:oIW9HwuWrhxv+4HZxA+QQSKHLqWFyXZ2FmNjUYwkdiM=

go/libraries/doltcore/sqle/dtables/workspace_table.go

Lines changed: 97 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"io"
2222

2323
"github.com/dolthub/go-mysql-server/sql"
24+
"github.com/dolthub/go-mysql-server/sql/analyzer"
25+
"github.com/dolthub/go-mysql-server/sql/plan"
2426
gmstypes "github.com/dolthub/go-mysql-server/sql/types"
2527
"github.com/dolthub/vitess/go/sqltypes"
2628

@@ -31,6 +33,7 @@ import (
3133
"github.com/dolthub/dolt/go/libraries/doltcore/schema"
3234
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
3335
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/index"
36+
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/overrides"
3437
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil"
3538
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/writer"
3639
"github.com/dolthub/dolt/go/libraries/doltcore/table/editor"
@@ -88,6 +91,9 @@ var _ sql.RowUpdater = (*WorkspaceTableUpdater)(nil)
8891

8992
type WorkspaceTableDeleter struct {
9093
WorkspaceTableModifier
94+
fkEditor *plan.ForeignKeyEditor
95+
fkUpdaters []sql.ForeignKeyEditor
96+
userTblEditor sql.ForeignKeyEditor
9197
}
9298

9399
var _ sql.RowDeleter = (*WorkspaceTableDeleter)(nil)
@@ -202,15 +208,42 @@ func (wtu *WorkspaceTableUpdater) Close(c *sql.Context) error {
202208
return nil
203209
}
204210

211+
// StatementBegin resolves the underlying table through the session catalog so
212+
// the revert flows through the same writer ordinary DML uses, then wraps it in
213+
// a ForeignKeyEditor so FK actions fire on the revert.
205214
func (wtd *WorkspaceTableDeleter) StatementBegin(ctx *sql.Context) {
206-
// Deletes are only allowed on WORKING, do not target staging.
207-
sessionWriter, tableWriter, err := wtd.getWorkspaceTableWriter(ctx, false)
215+
ds := dsess.DSessFromSess(ctx.Session)
216+
catalog := analyzer.NewCatalog(ds.GenericProvider(), overrides.EngineOverridesFromContext(ctx))
217+
catalog.AuthHandler = sql.NoopAuthorizationHandler{}
218+
219+
tbl, _, err := catalog.TableSchema(ctx, ctx.GetCurrentDatabase(), wtd.tableName.Schema, wtd.tableName.Name)
208220
if err != nil {
209221
wtd.err = &err
210222
return
211223
}
212-
wtd.tableWriter = &tableWriter
213-
wtd.sessionWriter = &sessionWriter
224+
fkTbl, ok := tbl.(sql.ForeignKeyTable)
225+
if !ok {
226+
e := fmt.Errorf("workspace target table %s does not support foreign keys", wtd.tableName)
227+
wtd.err = &e
228+
return
229+
}
230+
231+
fkEditor, updaters, err := analyzer.BuildForeignKeyEditor(ctx, catalog, fkTbl)
232+
if err != nil {
233+
wtd.err = &err
234+
return
235+
}
236+
237+
wtd.fkEditor = fkEditor
238+
wtd.fkUpdaters = updaters
239+
if fkEditor == nil {
240+
wtd.userTblEditor = fkTbl.GetForeignKeyEditor(ctx)
241+
wtd.userTblEditor.StatementBegin(ctx)
242+
return
243+
}
244+
for _, u := range updaters {
245+
u.StatementBegin(ctx)
246+
}
214247
}
215248

216249
func (wtd *WorkspaceTableDeleter) Delete(c *sql.Context, row sql.Row) error {
@@ -232,7 +265,6 @@ func (wtd *WorkspaceTableDeleter) Delete(c *sql.Context, row sql.Row) error {
232265
toRow := row[3 : 3+schemaLen]
233266
fromRow := row[3+schemaLen:]
234267

235-
// If to Row has any non-nil values, then we need to do an update. Otherwise, insert.
236268
wasDelete := true
237269
for _, val := range toRow {
238270
if val != nil {
@@ -248,25 +280,77 @@ func (wtd *WorkspaceTableDeleter) Delete(c *sql.Context, row sql.Row) error {
248280
}
249281
}
250282

251-
tableWriter := (*wtd.tableWriter)
252-
if tableWriter == nil {
253-
return fmt.Errorf("Runtime error: table writer is nil")
283+
if wtd.fkEditor != nil {
284+
switch {
285+
case wasInsert:
286+
return wtd.fkEditor.Delete(c, toRow, 0)
287+
case wasDelete:
288+
for _, ref := range wtd.fkEditor.References {
289+
if err := ref.CheckReference(c, fromRow); err != nil {
290+
return err
291+
}
292+
}
293+
return wtd.fkEditor.Editor.Insert(c, fromRow)
294+
default:
295+
return wtd.fkEditor.Update(c, toRow, fromRow, 0)
296+
}
254297
}
255298

299+
if wtd.userTblEditor == nil {
300+
return fmt.Errorf("Runtime error: table writer is nil")
301+
}
256302
if wasInsert {
257-
return tableWriter.Delete(c, toRow) // delete newly added row.
303+
return wtd.userTblEditor.Delete(c, toRow)
258304
} else if wasDelete {
259-
return tableWriter.Insert(c, fromRow) // restore deleted row.
305+
return wtd.userTblEditor.Insert(c, fromRow)
260306
} else {
261-
return tableWriter.Update(c, toRow, fromRow) // restore updated row.
307+
return wtd.userTblEditor.Update(c, toRow, fromRow)
262308
}
263309
}
264310

265311
func (wtd *WorkspaceTableDeleter) Close(c *sql.Context) error {
266-
// Resources released in StatementComplete
312+
var firstErr error
313+
for _, u := range wtd.fkUpdaters {
314+
if err := u.Close(c); err != nil && firstErr == nil {
315+
firstErr = err
316+
}
317+
}
318+
if wtd.userTblEditor != nil {
319+
if err := wtd.userTblEditor.Close(c); err != nil && firstErr == nil {
320+
firstErr = err
321+
}
322+
}
323+
return firstErr
324+
}
325+
326+
func (wtd *WorkspaceTableDeleter) DiscardChanges(ctx *sql.Context, errorEncountered error) error {
327+
for _, u := range wtd.fkUpdaters {
328+
if err := u.DiscardChanges(ctx, errorEncountered); err != nil {
329+
return err
330+
}
331+
}
332+
if wtd.userTblEditor != nil {
333+
return wtd.userTblEditor.DiscardChanges(ctx, errorEncountered)
334+
}
267335
return nil
268336
}
269337

338+
func (wtd *WorkspaceTableDeleter) StatementComplete(ctx *sql.Context) error {
339+
if wtd.err != nil {
340+
return *wtd.err
341+
}
342+
for _, u := range wtd.fkUpdaters {
343+
if err := u.StatementComplete(ctx); err != nil {
344+
return err
345+
}
346+
}
347+
if wtd.userTblEditor != nil {
348+
return wtd.userTblEditor.StatementComplete(ctx)
349+
}
350+
return nil
351+
}
352+
353+
270354
func (wtm *WorkspaceTableModifier) getWorkspaceTableWriter(ctx *sql.Context, targetStaging bool) (dsess.WriteSession, dsess.TableWriter, error) {
271355
ds := dsess.DSessFromSess(ctx.Session)
272356
var setter dsess.SessionRootSetter
@@ -417,7 +501,7 @@ func (wt *WorkspaceTable) Deleter(ctx *sql.Context) sql.RowDeleter {
417501
}
418502

419503
return &WorkspaceTableDeleter{
420-
modifier,
504+
WorkspaceTableModifier: modifier,
421505
}
422506
}
423507

go/libraries/doltcore/sqle/enginetest/dolt_queries_workspace.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,4 +1126,52 @@ var DoltWorkspaceScriptTests = []queries.ScriptTest{
11261126
},
11271127
},
11281128
},
1129+
{
1130+
// Regression for #11112.
1131+
Name: "dolt_workspace_* delete restricted by foreign key",
1132+
SetUpScript: []string{
1133+
"create table lists (id int primary key, name varchar(100));",
1134+
"create table list_entries (id int primary key, list_id int not null, content varchar(255), foreign key (list_id) references lists(id));",
1135+
"call dolt_commit('-Am', 'schema');",
1136+
"insert into lists values (1, 'list_1');",
1137+
"insert into list_entries values (1, 1, 'entry_1'), (2, 1, 'entry_2');",
1138+
},
1139+
Assertions: []queries.ScriptTestAssertion{
1140+
{
1141+
Query: "delete from dolt_workspace_lists where to_id = 1",
1142+
ExpectedErr: sql.ErrForeignKeyParentViolation,
1143+
},
1144+
{
1145+
Query: "select id from lists order by id",
1146+
Expected: []sql.Row{{1}},
1147+
},
1148+
{
1149+
Query: "select id, list_id from list_entries order by id",
1150+
Expected: []sql.Row{{1, 1}, {2, 1}},
1151+
},
1152+
},
1153+
},
1154+
{
1155+
Name: "dolt_workspace_* delete cascades through foreign key",
1156+
SetUpScript: []string{
1157+
"create table lists (id int primary key, name varchar(100));",
1158+
"create table list_entries (id int primary key, list_id int not null, content varchar(255), foreign key (list_id) references lists(id) on delete cascade);",
1159+
"call dolt_commit('-Am', 'schema');",
1160+
"insert into lists values (1, 'list_1');",
1161+
"insert into list_entries values (1, 1, 'entry_1'), (2, 1, 'entry_2');",
1162+
},
1163+
Assertions: []queries.ScriptTestAssertion{
1164+
{
1165+
Query: "delete from dolt_workspace_lists where to_id = 1",
1166+
},
1167+
{
1168+
Query: "select id from lists order by id",
1169+
Expected: []sql.Row{},
1170+
},
1171+
{
1172+
Query: "select id from list_entries order by id",
1173+
Expected: []sql.Row{},
1174+
},
1175+
},
1176+
},
11291177
}

0 commit comments

Comments
 (0)