From e3ff463c0fa04e3d1286638805e6bf8738059552 Mon Sep 17 00:00:00 2001 From: elianddb Date: Thu, 20 Nov 2025 16:59:29 -0800 Subject: [PATCH 1/7] add uninit subquery test for dolt/dolthub#10113 --- enginetest/memory_engine_test.go | 36 ++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index f1ec7b45d0..a3407c8ec1 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -200,23 +200,37 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - t.Skip() + //t.Skip() var scripts = []queries.ScriptTest{ { - Name: "AS OF propagates to nested CALLs", - SetUpScript: []string{}, + // 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: []queries.ScriptTestAssertion{ { - Query: "create procedure create_proc() create table t (i int primary key, j int);", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, + Query: "delete from student_hobby where not exists (select 1 from student where student.id = student_hobby.student_id);", }, { - Query: "call create_proc()", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, + Query: "SELECT * FROM student_hobby ORDER BY id;", + Expected: []sql.Row{{1, 1, "test1"}, {2, 2, "test2"}}, }, }, }, From d44af18bd4e305270d71d2b0535a74d677e0e783 Mon Sep 17 00:00:00 2001 From: elianddb Date: Tue, 25 Nov 2025 16:33:06 -0800 Subject: [PATCH 2/7] resolve subqueries in getBrachesForNode --- sql/analyzer/node_batches.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sql/analyzer/node_batches.go b/sql/analyzer/node_batches.go index 2f48169aa6..d14d26b350 100644 --- a/sql/analyzer/node_batches.go +++ b/sql/analyzer/node_batches.go @@ -107,6 +107,10 @@ func getBatchesForNode(node sql.Node) ([]*Batch, bool) { Id: validateReadOnlyTransactionId, Apply: validateReadOnlyTransaction, }, + { + Id: resolveSubqueriesId, + Apply: resolveSubqueries, + }, { Id: processTruncateId, Apply: processTruncate, From 49ecdfb06b093131040fd4bae8c487d0058f6501 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 26 Nov 2025 14:19:31 -0800 Subject: [PATCH 3/7] fix ExistsExpr buildScalar to use subquery impl --- enginetest/memory_engine_test.go | 36 +++++++++------------------- enginetest/queries/script_queries.go | 32 +++++++++++++++++++++++++ sql/analyzer/node_batches.go | 4 ---- sql/planbuilder/dml.go | 8 ++++++- sql/planbuilder/scalar.go | 14 +++++------ 5 files changed, 56 insertions(+), 38 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index a3407c8ec1..f1ec7b45d0 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -200,37 +200,23 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - //t.Skip() + t.Skip() var scripts = []queries.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');", - }, + Name: "AS OF propagates to nested CALLs", + SetUpScript: []string{}, Assertions: []queries.ScriptTestAssertion{ { - Query: "delete from student_hobby where not exists (select 1 from student where student.id = student_hobby.student_id);", + Query: "create procedure create_proc() create table t (i int primary key, j int);", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, }, { - Query: "SELECT * FROM student_hobby ORDER BY id;", - Expected: []sql.Row{{1, 1, "test1"}, {2, 2, "test2"}}, + Query: "call create_proc()", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, }, }, }, 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/node_batches.go b/sql/analyzer/node_batches.go index d14d26b350..2f48169aa6 100644 --- a/sql/analyzer/node_batches.go +++ b/sql/analyzer/node_batches.go @@ -107,10 +107,6 @@ func getBatchesForNode(node sql.Node) ([]*Batch, bool) { Id: validateReadOnlyTransactionId, Apply: validateReadOnlyTransaction, }, - { - Id: resolveSubqueriesId, - Apply: resolveSubqueries, - }, { Id: processTruncateId, Apply: processTruncate, diff --git a/sql/planbuilder/dml.go b/sql/planbuilder/dml.go index f49997e125..686c38fdd2 100644 --- a/sql/planbuilder/dml.go +++ b/sql/planbuilder/dml.go @@ -447,6 +447,13 @@ 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 + 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,7 +466,6 @@ 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 { targets = make([]sql.Node, len(d.Targets)) for i, tableName := range d.Targets { diff --git a/sql/planbuilder/scalar.go b/sql/planbuilder/scalar.go index 5d83706230..ac204f252b 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) + sq := b.buildScalar(inScope, v.Subquery) + sqPlan, ok := sq.(*plan.Subquery) + if !ok { + b.handleErr(fmt.Errorf("expected Subquery from ExistsExpr, got %T", sq)) + } + return plan.NewExistsSubquery(sqPlan) case *ast.TimestampFuncExpr: var ( unit sql.Expression From 8a0b70f41cacbff6573bdb2df042a94034d439e3 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 26 Nov 2025 14:25:43 -0800 Subject: [PATCH 4/7] fix names --- sql/planbuilder/scalar.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sql/planbuilder/scalar.go b/sql/planbuilder/scalar.go index ac204f252b..e352643161 100644 --- a/sql/planbuilder/scalar.go +++ b/sql/planbuilder/scalar.go @@ -372,12 +372,12 @@ func (b *Builder) buildScalar(inScope *scope, e ast.Expr) (ex sql.Expression) { return values } case *ast.ExistsExpr: - sq := b.buildScalar(inScope, v.Subquery) - sqPlan, ok := sq.(*plan.Subquery) + subquery := b.buildScalar(inScope, v.Subquery) + subqueryPlan, ok := subquery.(*plan.Subquery) if !ok { - b.handleErr(fmt.Errorf("expected Subquery from ExistsExpr, got %T", sq)) + b.handleErr(fmt.Errorf("expected Subquery from ExistsExpr, got %T", subquery)) } - return plan.NewExistsSubquery(sqPlan) + return plan.NewExistsSubquery(subqueryPlan) case *ast.TimestampFuncExpr: var ( unit sql.Expression From bcd456bfed274c50bbe40e7dd3a3d78830cc0e13 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 26 Nov 2025 14:52:46 -0800 Subject: [PATCH 5/7] add explicit targets flag to distinguish stored target --- sql/plan/delete.go | 42 +++++++++++++++---------------- sql/planbuilder/dml.go | 4 ++- sql/planbuilder/parse_old_test.go | 2 +- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/sql/plan/delete.go b/sql/plan/delete.go index 01270f97be..85fb9cfad0 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,33 @@ 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. +// 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 +99,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 +153,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 686c38fdd2..ef28377ac1 100644 --- a/sql/planbuilder/dml.go +++ b/sql/planbuilder/dml.go @@ -450,6 +450,7 @@ func (b *Builder) buildDelete(inScope *scope, d *ast.Delete) (outScope *scope) { // 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} } @@ -467,6 +468,7 @@ func (b *Builder) buildDelete(inScope *scope, d *ast.Delete) (outScope *scope) { } if len(d.Targets) > 0 { + hasExplicitTargets = true targets = make([]sql.Node, len(d.Targets)) for i, tableName := range d.Targets { tabName := tableName.Name.String() @@ -494,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"), From 6256206d0d5f1ae326ab0fc7a021b32b0df40f36 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 26 Nov 2025 15:06:17 -0800 Subject: [PATCH 6/7] use child on no targets --- sql/analyzer/apply_foreign_keys.go | 7 ++----- sql/plan/delete.go | 14 +++++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) 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/plan/delete.go b/sql/plan/delete.go index 85fb9cfad0..cf273bd353 100644 --- a/sql/plan/delete.go +++ b/sql/plan/delete.go @@ -70,9 +70,21 @@ func (p *DeleteFrom) WithExplicitTargets(targets []sql.Node) *DeleteFrom { return © } +// 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 { - return p.targets + if len(p.targets) > 0 { + return p.targets + } + return []sql.Node{p.Child} } // Schema implements the sql.Node interface. From ba9744213a5c4c269b5c8c097d6ff24d5ecc0867 Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 26 Nov 2025 16:18:19 -0800 Subject: [PATCH 7/7] rm fast path for virtual cols --- sql/analyzer/fix_exec_indexes.go | 2 +- sql/plan/delete.go | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) 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 cf273bd353..7ad69c2bb7 100644 --- a/sql/plan/delete.go +++ b/sql/plan/delete.go @@ -81,10 +81,7 @@ func (p *DeleteFrom) WithTargets(targets []sql.Node) *DeleteFrom { // GetDeleteTargets returns the sql.Nodes representing the tables from which rows should be deleted. func (p *DeleteFrom) GetDeleteTargets() []sql.Node { - if len(p.targets) > 0 { - return p.targets - } - return []sql.Node{p.Child} + return p.targets } // Schema implements the sql.Node interface.