Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions enginetest/queries/script_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 2 additions & 5 deletions sql/analyzer/apply_foreign_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sql/analyzer/fix_exec_indexes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
51 changes: 29 additions & 22 deletions sql/plan/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 10 additions & 2 deletions sql/planbuilder/dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sql/planbuilder/parse_old_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
14 changes: 6 additions & 8 deletions sql/planbuilder/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down