Skip to content

Commit d34f9de

Browse files
authored
fix update join with conflicting aliases (#3192)
1 parent 9aefd19 commit d34f9de

File tree

5 files changed

+58
-12
lines changed

5 files changed

+58
-12
lines changed

enginetest/queries/update_queries.go

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -597,9 +597,9 @@ var UpdateScriptTests = []ScriptTest{
597597
},
598598
},
599599
{
600-
Dialect: "mysql",
601600
// https://github.com/dolthub/dolt/issues/9403
602-
Name: "UPDATE join – multiple tables with same column names with triggers",
601+
Dialect: "mysql",
602+
Name: "UPDATE join – multiple tables with same column names with triggers",
603603
SetUpScript: []string{
604604
"create table customers (id int primary key, name text, tier text)",
605605
"create table orders (id int primary key, customer_id int, status text)",
@@ -632,8 +632,54 @@ var UpdateScriptTests = []ScriptTest{
632632
},
633633
},
634634
{
635-
Name: "UPDATE with subquery in keyless tables",
635+
Dialect: "mysql",
636+
Name: "UPDATE join - conflicting alias in Subquery Alias",
637+
SetUpScript: []string{
638+
"create table parent (id int primary key);",
639+
"insert into parent values (1), (2), (3);",
640+
"create table child (id int primary key, pid int, foreign key (pid) references parent(id), oid int);",
641+
"insert into child values (1, 1, 0), (2, 2, 0), (3, 3, 0);",
642+
},
643+
Assertions: []ScriptTestAssertion{
644+
{
645+
Query: `
646+
update child t1
647+
left join
648+
(
649+
select
650+
t1.id
651+
from
652+
child t1
653+
) sqa
654+
on
655+
t1.id = sqa.id
656+
join
657+
child t2
658+
set
659+
t1.oid = t2.pid;`,
660+
Expected: []sql.Row{
661+
{types.OkResult{
662+
RowsAffected: 3,
663+
Info: plan.UpdateInfo{
664+
Matched: 3,
665+
Updated: 3,
666+
},
667+
}},
668+
},
669+
},
670+
{
671+
Query: "select * from child;",
672+
Expected: []sql.Row{
673+
{1, 1, 1},
674+
{2, 2, 1},
675+
{3, 3, 1},
676+
},
677+
},
678+
},
679+
},
680+
{
636681
// https://github.com/dolthub/dolt/issues/9334
682+
Name: "UPDATE with subquery in keyless tables",
637683
SetUpScript: []string{
638684
"create table t (i int)",
639685
"insert into t values (1)",

sql/analyzer/apply_foreign_keys.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ func applyForeignKeysToNodes(ctx *sql.Context, a *Analyzer, n sql.Node, cache *f
127127
fkHandlerMap := make(map[string]sql.Node, len(updateTargets))
128128
for tableName, updateTarget := range updateTargets {
129129
fkHandlerMap[tableName] = updateTarget
130-
fkHandler, err :=
131-
getForeignKeyHandlerFromUpdateTarget(ctx, a, updateTarget, cache, fkChain)
130+
fkHandler, err := getForeignKeyHandlerFromUpdateTarget(ctx, a, updateTarget, cache, fkChain)
132131
if err != nil {
133132
return nil, transform.SameTree, err
134133
}

sql/analyzer/tables.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,28 @@ func getResolvedTable(node sql.Node) *plan.ResolvedTable {
9898
}
9999

100100
// getTablesByName takes a node and returns all found resolved tables in a map.
101+
// This function will not look inside sql.OpaqueNodes (like plan.SubqueryAlias).
101102
func getTablesByName(node sql.Node) map[string]*plan.ResolvedTable {
102103
ret := make(map[string]*plan.ResolvedTable)
103-
104-
transform.Inspect(node, func(node sql.Node) bool {
104+
// TODO: We should change transform.Inspect to not walk the children of sql.OpaqueNodes (like transform.Node)
105+
// and add a transform.InspectWithOpaque that does.
106+
// Using transform.Node here achieves the same result without a large refactor.
107+
transform.Node(node, func(node sql.Node) (sql.Node, transform.TreeIdentity, error) {
105108
switch n := node.(type) {
106109
case *plan.ResolvedTable:
107110
ret[strings.ToLower(n.Table.Name())] = n
108111
case *plan.IndexedTableAccess:
109112
rt, ok := n.TableNode.(*plan.ResolvedTable)
110113
if ok {
111114
ret[strings.ToLower(rt.Name())] = rt
112-
return false
113115
}
114116
case *plan.TableAlias:
115117
rt := getResolvedTable(n)
116118
if rt != nil {
117119
ret[n.Name()] = rt
118120
}
119-
default:
120121
}
121-
return true
122+
return nil, transform.SameTree, nil
122123
})
123-
124124
return ret
125125
}

sql/plan/common.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func getTableName(nodeToSearch sql.Node) string {
164164
return ""
165165
}
166166

167-
// GetTablesToBeUpdated takes a node and looks for the tables to modified by a SetField.
167+
// GetTablesToBeUpdated takes a node and looks for the tables modified by a SetField.
168168
func GetTablesToBeUpdated(node sql.Node) map[string]struct{} {
169169
ret := make(map[string]struct{})
170170

sql/plan/subqueryalias.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ type SubqueryAlias struct {
4444
var _ sql.Node = (*SubqueryAlias)(nil)
4545
var _ sql.CollationCoercible = (*SubqueryAlias)(nil)
4646
var _ sql.RenameableNode = (*SubqueryAlias)(nil)
47+
var _ sql.OpaqueNode = (*SubqueryAlias)(nil)
4748

4849
// NewSubqueryAlias creates a new SubqueryAlias node.
4950
func NewSubqueryAlias(name, textDefinition string, node sql.Node) *SubqueryAlias {

0 commit comments

Comments
 (0)