diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index 1f8d9d9769..886da7a9c6 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -122,6 +122,38 @@ type ScriptTestAssertion struct { // Unlike other engine tests, ScriptTests must be self-contained. No other tables are created outside the definition of // the tests. var ScriptTests = []ScriptTest{ + { + // https://github.com/dolthub/dolt/issues/10113 + Name: "DELETE with NOT EXISTS subquery", + SetUpScript: []string{ + `CREATE TABLE IF NOT EXISTS student ( + id BIGINT AUTO_INCREMENT, + name VARCHAR(50) NOT NULL, + PRIMARY KEY (id) + );`, + `CREATE TABLE IF NOT EXISTS student_hobby ( + id BIGINT AUTO_INCREMENT, + student_id BIGINT NOT NULL, + hobby VARCHAR(50) NOT NULL, + PRIMARY KEY (id) + );`, + "INSERT INTO student (id, name) VALUES (1, 'test1');", + "INSERT INTO student (id, name) VALUES (2, 'test2');", + "INSERT INTO student_hobby (id, student_id, hobby) VALUES (1, 1, 'test1');", + "INSERT INTO student_hobby (id, student_id, hobby) VALUES (2, 2, 'test2');", + "INSERT INTO student_hobby (id, student_id, hobby) VALUES (3, 100, 'test3');", + "INSERT INTO student_hobby (id, student_id, hobby) VALUES (4, 100, 'test3');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "delete from student_hobby where not exists (select 1 from student where student.id = student_hobby.student_id);", + }, + { + Query: "SELECT * FROM student_hobby ORDER BY id;", + Expected: []sql.Row{{1, 1, "test1"}, {2, 2, "test2"}}, + }, + }, + }, { // https://github.com/dolthub/dolt/issues/9987 Name: "GROUP BY nil pointer dereference in Dispose when Next() never called", diff --git a/sql/analyzer/apply_foreign_keys.go b/sql/analyzer/apply_foreign_keys.go index f45667e871..5fddbd11fe 100644 --- a/sql/analyzer/apply_foreign_keys.go +++ b/sql/analyzer/apply_foreign_keys.go @@ -189,11 +189,8 @@ func applyForeignKeysToNodes(ctx *sql.Context, a *Analyzer, n sql.Node, cache *f if n.HasExplicitTargets() { return n.WithExplicitTargets(foreignKeyHandlers), transform.NewTree, nil } else { - newNode, err := n.WithChildren(foreignKeyHandlers...) - if err != nil { - return nil, transform.SameTree, err - } - return newNode, transform.NewTree, nil + // For simple DELETEs, update the targets array with foreign key handlers. + return n.WithTargets(foreignKeyHandlers), transform.NewTree, nil } default: return n, transform.SameTree, nil diff --git a/sql/analyzer/fix_exec_indexes.go b/sql/analyzer/fix_exec_indexes.go index 1b239345f0..62dd4d6a8c 100644 --- a/sql/analyzer/fix_exec_indexes.go +++ b/sql/analyzer/fix_exec_indexes.go @@ -53,7 +53,7 @@ func assignExecIndexes(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Sc } } case *plan.DeleteFrom: - if n.RefsSingleRel && !n.HasExplicitTargets() && scope.IsEmpty() && !n.IsProcNested { + if n.RefsSingleRel && !n.HasExplicitTargets() && scope.IsEmpty() && !n.IsProcNested && !n.Child.Schema().HasVirtualColumns() { // joins, subqueries, triggers, and procedures preclude fast indexing return offsetAssignIndexes(n), transform.NewTree, nil } diff --git a/sql/plan/delete.go b/sql/plan/delete.go index 01270f97be..7ad69c2bb7 100644 --- a/sql/plan/delete.go +++ b/sql/plan/delete.go @@ -28,10 +28,12 @@ var ErrDeleteFromNotSupported = errors.NewKind("table doesn't support DELETE FRO // DeleteFrom is a node describing a deletion from some table. type DeleteFrom struct { UnaryNode - // targets are the explicitly specified table nodes from which rows should be deleted. For simple DELETES against a - // single source table, targets do NOT need to be explicitly specified and will not be set here. For DELETE FROM JOIN - // statements, targets MUST be explicitly specified by the user and will be populated here. - explicitTargets []sql.Node + // targets contains the table nodes from which rows should be deleted. For simple DELETEs, this contains the single + // inferred table. For DELETE FROM JOIN, this contains the explicitly specified tables. + targets []sql.Node + // hasExplicitTargets indicates whether the targets were explicitly specified in SQL (e.g., "DELETE t1, t2 FROM ... + // ") vs inferred (e.g., "DELETE FROM table WHERE ..."). + hasExplicitTargets bool // Returning is a list of expressions to return after the delete operation. This feature is not // supported in MySQL's syntax, but is exposed through PostgreSQL's syntax. Returning []sql.Expression @@ -44,37 +46,42 @@ var _ sql.Node = (*DeleteFrom)(nil) var _ sql.CollationCoercible = (*DeleteFrom)(nil) // NewDeleteFrom creates a DeleteFrom node. -func NewDeleteFrom(n sql.Node, targets []sql.Node) *DeleteFrom { +func NewDeleteFrom(n sql.Node, targets []sql.Node, hasExplicitTargets bool) *DeleteFrom { return &DeleteFrom{ - UnaryNode: UnaryNode{n}, - explicitTargets: targets, + UnaryNode: UnaryNode{n}, + targets: targets, + hasExplicitTargets: hasExplicitTargets, } } -// HasExplicitTargets returns true if the target delete tables were explicitly specified. This can only happen with -// DELETE FROM JOIN statements – for DELETE FROM statements using a single source table, the target is NOT explicitly -// specified and is assumed to be the single source table. +// HasExplicitTargets returns true if the target delete tables were explicitly specified in SQL. This can only happen +// with DELETE FROM JOIN statements. For DELETE FROM statements using a single source table, the target is NOT +// explicitly specified and is assumed to be the single source table. func (p *DeleteFrom) HasExplicitTargets() bool { - return len(p.explicitTargets) > 0 + return p.hasExplicitTargets } // WithExplicitTargets returns a new DeleteFrom node instance with the specified |targets| set as the explicitly // specified targets of the delete operation. func (p *DeleteFrom) WithExplicitTargets(targets []sql.Node) *DeleteFrom { copy := *p - copy.explicitTargets = targets + copy.targets = targets + copy.hasExplicitTargets = true return © } -// GetDeleteTargets returns the sql.Nodes representing the tables from which rows should be deleted. For a DELETE FROM -// JOIN statement, this will return the tables explicitly specified by the caller. For a DELETE FROM statement this will -// return the single table in the DELETE FROM source that is implicitly assumed to be the target of the delete operation. +// WithTargets returns a new DeleteFrom node instance with the specified |targets| set, preserving the +// hasExplicitTargets flag. This is used for simple DELETEs where targets need to be updated (e.g., with +// foreign key handlers) without changing whether they were explicitly specified. +func (p *DeleteFrom) WithTargets(targets []sql.Node) *DeleteFrom { + copy := *p + copy.targets = targets + return © +} + +// GetDeleteTargets returns the sql.Nodes representing the tables from which rows should be deleted. func (p *DeleteFrom) GetDeleteTargets() []sql.Node { - if len(p.explicitTargets) == 0 { - return []sql.Node{p.Child} - } else { - return p.explicitTargets - } + return p.targets } // Schema implements the sql.Node interface. @@ -101,7 +108,7 @@ func (p *DeleteFrom) Resolved() bool { return false } - for _, target := range p.explicitTargets { + for _, target := range p.targets { if target.Resolved() == false { return false } @@ -155,7 +162,7 @@ func (p *DeleteFrom) WithChildren(children ...sql.Node) (sql.Node, error) { return nil, sql.ErrInvalidChildrenNumber.New(p, len(children), 1) } - deleteFrom := NewDeleteFrom(children[0], p.explicitTargets) + deleteFrom := NewDeleteFrom(children[0], p.targets, p.hasExplicitTargets) deleteFrom.Returning = p.Returning return deleteFrom, nil } diff --git a/sql/planbuilder/dml.go b/sql/planbuilder/dml.go index f49997e125..ef28377ac1 100644 --- a/sql/planbuilder/dml.go +++ b/sql/planbuilder/dml.go @@ -447,6 +447,14 @@ func (b *Builder) buildDelete(inScope *scope, d *ast.Delete) (outScope *scope) { b.qFlags.Set(sql.QFlagDelete) outScope = b.buildFrom(inScope, d.TableExprs) + + // Capture the table node for simple DELETEs before buildWhere wraps it + var targets []sql.Node + var hasExplicitTargets bool + if len(d.Targets) == 0 { + targets = []sql.Node{outScope.node} + } + b.buildWhere(outScope, d.Where) orderByScope := b.analyzeOrderBy(outScope, outScope, d.OrderBy) b.buildOrderBy(outScope, orderByScope) @@ -459,8 +467,8 @@ func (b *Builder) buildDelete(inScope *scope, d *ast.Delete) (outScope *scope) { outScope.node = plan.NewLimit(limit, outScope.node) } - var targets []sql.Node if len(d.Targets) > 0 { + hasExplicitTargets = true targets = make([]sql.Node, len(d.Targets)) for i, tableName := range d.Targets { tabName := tableName.Name.String() @@ -488,7 +496,7 @@ func (b *Builder) buildDelete(inScope *scope, d *ast.Delete) (outScope *scope) { } } - del := plan.NewDeleteFrom(outScope.node, targets) + del := plan.NewDeleteFrom(outScope.node, targets, hasExplicitTargets) del.RefsSingleRel = !outScope.refsSubquery del.IsProcNested = b.ProcCtx().DbName != "" outScope.node = del diff --git a/sql/planbuilder/parse_old_test.go b/sql/planbuilder/parse_old_test.go index 66906fea55..6fc698bcbf 100644 --- a/sql/planbuilder/parse_old_test.go +++ b/sql/planbuilder/parse_old_test.go @@ -5082,7 +5082,7 @@ func TestParseCreateTrigger(t *testing.T) { plan.NewFilter( expression.NewEquals(expression.NewUnresolvedColumn("a"), expression.NewUnresolvedQualifiedColumn("old", "b")), plan.NewUnresolvedTable("baz", ""), - ), nil), + ), nil, false), plan.NewInsertInto(sql.UnresolvedDatabase(""), plan.NewUnresolvedTable("zzz", ""), plan.NewValues([][]sql.Expression{{ expression.NewUnresolvedQualifiedColumn("old", "a"), expression.NewUnresolvedQualifiedColumn("old", "b"), diff --git a/sql/planbuilder/scalar.go b/sql/planbuilder/scalar.go index 5d83706230..e352643161 100644 --- a/sql/planbuilder/scalar.go +++ b/sql/planbuilder/scalar.go @@ -372,14 +372,12 @@ func (b *Builder) buildScalar(inScope *scope, e ast.Expr) (ex sql.Expression) { return values } case *ast.ExistsExpr: - sqScope := inScope.push() - sqScope.initSubquery() - selScope := b.buildSelectStmt(sqScope, v.Subquery.Select) - selectString := ast.String(v.Subquery.Select) - sq := plan.NewSubquery(selScope.node, selectString) - sq = sq.WithCorrelated(sqScope.correlated()) - b.qFlags.Set(sql.QFlagScalarSubquery) - return plan.NewExistsSubquery(sq) + subquery := b.buildScalar(inScope, v.Subquery) + subqueryPlan, ok := subquery.(*plan.Subquery) + if !ok { + b.handleErr(fmt.Errorf("expected Subquery from ExistsExpr, got %T", subquery)) + } + return plan.NewExistsSubquery(subqueryPlan) case *ast.TimestampFuncExpr: var ( unit sql.Expression