Merge pull request #3318 from dolthub/elian/10113

dolthub/dolt#10113: Fix DELETE queries with NOT EXISTS uninitialized subqueries

Author: elianddb

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",

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

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
 		}

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 &copy
 }
 
-// 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 &copy
+}
+
+// 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
 }

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

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"),

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