Skip to content

Commit 12fb79e

Browse files
committed
added check for if schema contains columns with the same name
1 parent 9882720 commit 12fb79e

File tree

3 files changed

+101
-4
lines changed

3 files changed

+101
-4
lines changed

enginetest/queries/script_queries.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,15 +514,15 @@ SET entity_test.value = joined.value;`,
514514
Expected: []sql.Row{{1, "john", "doe", 0, 42}},
515515
},
516516
{
517-
Query: "UPDATE test_users JOIN (SELECT id, 1 FROM test_users) AS tu SET test_users.favorite_number = 420;",
517+
Query: "UPDATE test_users JOIN (SELECT 1 FROM test_users) AS tu SET test_users.favorite_number = 420;",
518518
Expected: []sql.Row{{NewUpdateResult(1, 1)}},
519519
},
520520
{
521521
Query: "select * from test_users;",
522522
Expected: []sql.Row{{1, "john", "doe", 0, 420}},
523523
},
524524
{
525-
Query: "UPDATE test_users JOIN (SELECT id, 1 FROM test_users) AS tu SET test_users.deleted = 1;",
525+
Query: "UPDATE test_users JOIN (SELECT 1 FROM test_users) AS tu SET test_users.deleted = 1;",
526526
Expected: []sql.Row{{NewUpdateResult(1, 1)}},
527527
},
528528
{

enginetest/queries/update_queries.go

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ var UpdateScriptTests = []ScriptTest{
534534
Name: "UPDATE join – multiple tables, with trigger",
535535
SetUpScript: []string{
536536
"CREATE TABLE a (id INT PRIMARY KEY, x INT);",
537-
"CREATE TABLE b (id INT PRIMARY KEY, y INT);",
537+
"CREATE TABLE b (pk INT PRIMARY KEY, y INT);",
538538
"CREATE TABLE logbook (entry TEXT);",
539539
`CREATE TRIGGER trig_a AFTER UPDATE ON a FOR EACH ROW
540540
BEGIN
@@ -550,7 +550,7 @@ var UpdateScriptTests = []ScriptTest{
550550
Assertions: []ScriptTestAssertion{
551551
{
552552
Query: `UPDATE a
553-
JOIN b ON a.id = 5 AND b.id = 6
553+
JOIN b ON a.id = 5 AND b.pk = 6
554554
SET a.x = 101, b.y = 201;`,
555555
},
556556
{
@@ -562,6 +562,82 @@ var UpdateScriptTests = []ScriptTest{
562562
},
563563
},
564564
},
565+
{
566+
Dialect: "mysql",
567+
Name: "UPDATE join – multiple tables with triggers that reference row values",
568+
SetUpScript: []string{
569+
"create table customers (id int primary key, name text, tier text)",
570+
"create table orders (order_id int primary key, customer_id int, status text)",
571+
"create table trigger_log (msg text)",
572+
`CREATE TRIGGER after_orders_update after update on orders for each row
573+
begin
574+
insert into trigger_log (msg) values(
575+
concat('Order ', OLD.order_id, ' status changed from ', OLD.status, ' to ', NEW.status));
576+
end;`,
577+
`Create trigger after_customers_update after update on customers for each row
578+
begin
579+
insert into trigger_log (msg) values(
580+
concat('Customer ', OLD.id, ' tier changed from ', OLD.tier, ' to ', NEW.tier));
581+
end;`,
582+
"insert into customers values(1, 'Alice', 'silver'), (2, 'Bob', 'gold');",
583+
"insert into orders values (101, 1, 'pending'), (102, 2, 'pending');",
584+
"update customers c join orders o on c.id = o.customer_id " +
585+
"set c.tier = 'platinum', o.status = 'shipped' where o.status = 'pending'",
586+
},
587+
Assertions: []ScriptTestAssertion{
588+
{
589+
Query: "SELECT * FROM trigger_log order by msg;",
590+
Expected: []sql.Row{
591+
{"Customer 1 tier changed from silver to platinum"},
592+
{"Customer 2 tier changed from gold to platinum"},
593+
{"Order 101 status changed from pending to shipped"},
594+
{"Order 102 status changed from pending to shipped"},
595+
},
596+
},
597+
},
598+
},
599+
{
600+
Dialect: "mysql",
601+
Name: "UPDATE join – multiple tables with same column names with triggers",
602+
SetUpScript: []string{
603+
"create table customers (id int primary key, name text, tier text)",
604+
"create table orders (id int primary key, customer_id int, status text)",
605+
"create table trigger_log (msg text)",
606+
`CREATE TRIGGER after_orders_update after update on orders for each row
607+
begin
608+
insert into trigger_log (msg) values(
609+
concat('Order ', OLD.id, ' status changed from ', OLD.status, ' to ', NEW.status));
610+
end;`,
611+
`Create trigger after_customers_update after update on customers for each row
612+
begin
613+
insert into trigger_log (msg) values(
614+
concat('Customer ', OLD.id, ' tier changed from ', OLD.tier, ' to ', NEW.tier));
615+
end;`,
616+
"insert into customers values(1, 'Alice', 'silver'), (2, 'Bob', 'gold');",
617+
"insert into orders values (101, 1, 'pending'), (102, 2, 'pending');",
618+
},
619+
Assertions: []ScriptTestAssertion{
620+
{
621+
Query: "update customers c join orders o on c.id = o.customer_id " +
622+
"set c.tier = 'platinum', o.status = 'shipped' where o.status = 'pending'",
623+
// TODO: we shouldn't expect an error once we're able to handle conflicting column names
624+
// https://github.com/dolthub/dolt/issues/9403
625+
ExpectedErrStr: "Unable to apply triggers when joined tables have columns with the same name",
626+
},
627+
{
628+
// TODO: unskip once we're able to handle conflicting column names
629+
// https://github.com/dolthub/dolt/issues/9403
630+
Skip: true,
631+
Query: "SELECT * FROM trigger_log order by msg;",
632+
Expected: []sql.Row{
633+
{"Customer 1 tier changed from silver to platinum"},
634+
{"Customer 2 tier changed from gold to platinum"},
635+
{"Order 101 status changed from pending to shipped"},
636+
{"Order 102 status changed from pending to shipped"},
637+
},
638+
},
639+
},
640+
},
565641
}
566642

567643
var SpatialUpdateTests = []WriteQueryTest{

sql/analyzer/triggers.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package analyzer
1616

1717
import (
18+
"errors"
1819
"fmt"
1920
"strings"
2021

@@ -479,6 +480,12 @@ func getTriggerLogic(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
479480
),
480481
)
481482
} else {
483+
// TODO: We should be able to handle duplicate column names by masking columns that aren't part of the
484+
// triggered table https://github.com/dolthub/dolt/issues/9403
485+
err = validateNoConflictingColumnNames(updateSrc.Child)
486+
if err != nil {
487+
return nil, err
488+
}
482489
// The scopeNode for an UpdateJoin should contain every node in the updateSource as new and old.
483490
scopeNode = plan.NewProject(
484491
[]sql.Expression{expression.NewStar()},
@@ -504,6 +511,20 @@ func getTriggerLogic(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop
504511
return triggerLogic, err
505512
}
506513

514+
// validateNoConflictingColumnNames checks the columns of a joined table to make sure there are no conflicting column
515+
// names
516+
func validateNoConflictingColumnNames(n sql.Node) error {
517+
sch := n.Schema()
518+
columnNames := make(map[string]string)
519+
for _, col := range sch {
520+
if sourceName, ok := columnNames[col.Name]; ok && sourceName != col.Source {
521+
return errors.New("Unable to apply triggers when joined tables have columns with the same name")
522+
}
523+
columnNames[col.Name] = col.Source
524+
}
525+
return nil
526+
}
527+
507528
// validateNoCircularUpdates returns an error if the trigger logic attempts to update the table that invoked it (or any
508529
// table being updated in an outer scope of this analysis)
509530
func validateNoCircularUpdates(trigger *plan.CreateTrigger, n sql.Node, scope *plan.Scope) error {

0 commit comments

Comments
 (0)