From 77d425dd3ed8011cede6a324315bbf2b39c6eb1d Mon Sep 17 00:00:00 2001 From: Max Hoffman Date: Wed, 26 Mar 2025 16:33:04 -0700 Subject: [PATCH 1/5] insert trigger bugs --- enginetest/queries/trigger_queries.go | 44 +++++++++++++++++++++++++++ sql/analyzer/fix_exec_indexes.go | 5 +++ sql/rowexec/dml_iters.go | 21 +++++++++++++ 3 files changed, 70 insertions(+) diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index 83ed0d5e18..ee9ca75466 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -310,6 +310,50 @@ var TriggerTests = []ScriptTest{ }, }, }, + { + Name: "issue #9039: trigger join index error subquery", + SetUpScript: []string{ + "create table a (x int primary key, y int default 1, z int)", + "create table b (x int primary key)", + "create table c (x int primary key)", + "create table d (x int primary key)", + "insert into b values (1), (2)", + "insert into d values (1), (2)", + ` +create trigger insert_into_a +after insert on a +for each row replace into c +select * from d join (select * from b where new.x = b.x) as e using (x) +where d.x = new.x`, + "insert into a (x,z) values (2,2)", + }, + Query: "select x from c order by 1", + Expected: []sql.Row{ + {2}, + }, + }, + { + Name: "issue #9039: trigger join index error", + SetUpScript: []string{ + "create table a (x int primary key, y int default 1, z int)", + "create table b (x int primary key)", + "create table c (x int primary key)", + "create table d (x int primary key)", + "insert into b values (1), (2)", + "insert into d values (1), (2)", + ` +create trigger insert_into_a +after insert on a +for each row replace into c +select * from d join b where new.x = b.x using (x) +where d.x = new.x`, + "insert into a (x,z) values (2,2)", + }, + Query: "select x from c order by 1", + Expected: []sql.Row{ + {2}, + }, + }, { Name: "trigger before insert, alter inserted value", SetUpScript: []string{ diff --git a/sql/analyzer/fix_exec_indexes.go b/sql/analyzer/fix_exec_indexes.go index 2c30e3db94..0764666e26 100644 --- a/sql/analyzer/fix_exec_indexes.go +++ b/sql/analyzer/fix_exec_indexes.go @@ -312,6 +312,11 @@ func (s *idxScope) visitChildren(n sql.Node) error { // join subquery aliases continue to enjoy full visibility. sqScope.parentScopes = sqScope.parentScopes[:0] sqScope.lateralScopes = sqScope.lateralScopes[:0] + for _, p := range s.parentScopes { + if p.triggerScope { + sqScope.parentScopes = append(sqScope.parentScopes, p) + } + } } newC, cScope, err := assignIndexesHelper(n.Child, sqScope) if err != nil { diff --git a/sql/rowexec/dml_iters.go b/sql/rowexec/dml_iters.go index 57dfd72555..e102c74536 100644 --- a/sql/rowexec/dml_iters.go +++ b/sql/rowexec/dml_iters.go @@ -17,6 +17,7 @@ package rowexec import ( "fmt" "io" + "log" "sync" "github.com/dolthub/go-mysql-server/sql" @@ -209,6 +210,24 @@ func prependRowInPlanForTriggerExecution(row sql.Row) func(c transform.Context) return n, transform.SameTree, nil } return n.WithProcedure(newNode.(*plan.Procedure)), transform.NewTree, nil + case *plan.InsertInto: + newNode, same, err := transform.NodeWithCtx(n.Source, prependRowForTriggerExecutionSelector, prependRowInPlanForTriggerExecution(row)) + if err != nil { + return nil, transform.SameTree, err + } + if same { + return n, transform.SameTree, nil + } + return n.WithSource(newNode), transform.NewTree, nil + case *plan.SubqueryAlias: + newNode, same, err := transform.NodeWithCtx(n.Child, prependRowForTriggerExecutionSelector, prependRowInPlanForTriggerExecution(row)) + if err != nil { + return nil, transform.SameTree, err + } + if same { + return n, transform.SameTree, nil + } + return n.WithChild(newNode), transform.NewTree, nil default: return n, transform.SameTree, nil } @@ -237,6 +256,8 @@ func (t *triggerIter) Next(ctx *sql.Context) (row sql.Row, returnErr error) { return nil, err } + log.Println(sql.DebugString(logic)) + // We don't do anything interesting with this subcontext yet, but it's a good idea to cancel it independently of the // parent context if something goes wrong in trigger execution. ctx, cancelFunc := t.ctx.NewSubContext() From aecdf1ff996d772eeb630332957c1a4c8405f0f6 Mon Sep 17 00:00:00 2001 From: Max Hoffman Date: Thu, 27 Mar 2025 10:20:37 -0700 Subject: [PATCH 2/5] more bugs --- enginetest/queries/trigger_queries.go | 28 ++++++++++++++++++++++++--- sql/rowexec/dml_iters.go | 2 +- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index ee9ca75466..b13296c6d3 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -311,7 +311,7 @@ var TriggerTests = []ScriptTest{ }, }, { - Name: "issue #9039: trigger join index error subquery", + Name: "issue #9039: trigger insert subquery error", SetUpScript: []string{ "create table a (x int primary key, y int default 1, z int)", "create table b (x int primary key)", @@ -333,7 +333,7 @@ where d.x = new.x`, }, }, { - Name: "issue #9039: trigger join index error", + Name: "issue #9039: trigger insert join index error", SetUpScript: []string{ "create table a (x int primary key, y int default 1, z int)", "create table b (x int primary key)", @@ -345,7 +345,7 @@ where d.x = new.x`, create trigger insert_into_a after insert on a for each row replace into c -select * from d join b where new.x = b.x using (x) +select * from d join b using (x) where d.x = new.x`, "insert into a (x,z) values (2,2)", }, @@ -354,6 +354,28 @@ where d.x = new.x`, {2}, }, }, + { + Name: "issue #9039: trigger insert projection index error", + SetUpScript: []string{ + "create table a (x int primary key, y int default 1, z int)", + "create table b (x int primary key)", + "create table c (x int primary key, y tinyint)", + "create table d (x int primary key)", + "insert into b values (1), (2)", + "insert into d values (1), (2)", + ` +create trigger insert_into_a +after insert on a +for each row replace into c +select d.x+2, 0 from d join b using (x) +where d.x = new.x`, + "insert into a (x,z) values (2,2)", + }, + Query: "select x, y from c order by 1", + Expected: []sql.Row{ + {4, 0}, + }, + }, { Name: "trigger before insert, alter inserted value", SetUpScript: []string{ diff --git a/sql/rowexec/dml_iters.go b/sql/rowexec/dml_iters.go index e102c74536..f87bb0ad7b 100644 --- a/sql/rowexec/dml_iters.go +++ b/sql/rowexec/dml_iters.go @@ -194,7 +194,7 @@ func prependRowInPlanForTriggerExecution(row sql.Row) func(c transform.Context) case *plan.Project: // Only prepend rows for projects that aren't the input to inserts and other triggers switch c.Parent.(type) { - case *plan.InsertInto, *plan.Into, *plan.TriggerExecutor, *plan.DeclareCursor: + case *plan.InsertInto, *plan.Into, *plan.TriggerExecutor, *plan.DeclareCursor, *plan.Project: return n, transform.SameTree, nil default: return plan.NewPrependNode(n, row), transform.NewTree, nil From c1826672d9a14336fdcc20c256a73de1f523ea32 Mon Sep 17 00:00:00 2001 From: Max Hoffman Date: Thu, 27 Mar 2025 10:28:53 -0700 Subject: [PATCH 3/5] delete debug line --- sql/rowexec/dml_iters.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sql/rowexec/dml_iters.go b/sql/rowexec/dml_iters.go index f87bb0ad7b..428d2b9a03 100644 --- a/sql/rowexec/dml_iters.go +++ b/sql/rowexec/dml_iters.go @@ -255,9 +255,7 @@ func (t *triggerIter) Next(ctx *sql.Context) (row sql.Row, returnErr error) { if err != nil { return nil, err } - - log.Println(sql.DebugString(logic)) - + // We don't do anything interesting with this subcontext yet, but it's a good idea to cancel it independently of the // parent context if something goes wrong in trigger execution. ctx, cancelFunc := t.ctx.NewSubContext() From a097f8a926c60f93cf72e921012844d394b9a368 Mon Sep 17 00:00:00 2001 From: max-hoffman Date: Thu, 27 Mar 2025 17:30:23 +0000 Subject: [PATCH 4/5] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/rowexec/dml_iters.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sql/rowexec/dml_iters.go b/sql/rowexec/dml_iters.go index 428d2b9a03..6da48ac8cf 100644 --- a/sql/rowexec/dml_iters.go +++ b/sql/rowexec/dml_iters.go @@ -17,7 +17,6 @@ package rowexec import ( "fmt" "io" - "log" "sync" "github.com/dolthub/go-mysql-server/sql" @@ -255,7 +254,7 @@ func (t *triggerIter) Next(ctx *sql.Context) (row sql.Row, returnErr error) { if err != nil { return nil, err } - + // We don't do anything interesting with this subcontext yet, but it's a good idea to cancel it independently of the // parent context if something goes wrong in trigger execution. ctx, cancelFunc := t.ctx.NewSubContext() From 79302ec9d2fbe90c00ea638b850ae8a8d2b168c6 Mon Sep 17 00:00:00 2001 From: Max Hoffman Date: Thu, 27 Mar 2025 12:29:55 -0700 Subject: [PATCH 5/5] unused import --- sql/rowexec/dml_iters.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sql/rowexec/dml_iters.go b/sql/rowexec/dml_iters.go index 428d2b9a03..6da48ac8cf 100644 --- a/sql/rowexec/dml_iters.go +++ b/sql/rowexec/dml_iters.go @@ -17,7 +17,6 @@ package rowexec import ( "fmt" "io" - "log" "sync" "github.com/dolthub/go-mysql-server/sql" @@ -255,7 +254,7 @@ func (t *triggerIter) Next(ctx *sql.Context) (row sql.Row, returnErr error) { if err != nil { return nil, err } - + // We don't do anything interesting with this subcontext yet, but it's a good idea to cancel it independently of the // parent context if something goes wrong in trigger execution. ctx, cancelFunc := t.ctx.NewSubContext()