From 9352f808e6c1cb2abe5451a11db858958712f77c Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 9 Apr 2024 14:26:47 -0700 Subject: [PATCH 1/5] prevent panic --- enginetest/queries/trigger_queries.go | 40 +++++++++++++++++++++++++++ sql/expression/procedurereference.go | 6 +++- sql/plan/declare_variables.go | 1 + 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index d0488ad2b1..15a28faad6 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -2204,6 +2204,46 @@ INSERT INTO t0 (v1, v2) VALUES (i, s); END;`, }, }, }, + + { + Name: "Trigger with BLOCK and DECLARE", + SetUpScript: []string{ + "create table t (i int primary key);", + "create trigger trig before insert on t for each row begin declare x int; select new.i + 10 into x; set new.i = x; end;", + //"create trigger trig before insert on t for each row begin declare x int; set x = new.i * 10; set new.i = x; end;", + }, + Assertions: []ScriptTestAssertion{ + { + Skip: true, + Query: "insert into t values (1);", + Expected: []sql.Row{ + {types.NewOkResult(1)}, + }, + }, + { + Skip: true, + Query: "insert into t values (2);", + Expected: []sql.Row{ + {types.NewOkResult(1)}, + }, + }, + { + Skip: true, + Query: "select * from t;", + Expected: []sql.Row{ + {10}, + }, + }, + { + Skip: true, + Query: "select * from t;", + Expected: []sql.Row{ + {20}, + }, + }, + }, + }, + } // RollbackTriggerTests are trigger tests that require rollback logic to work correctly diff --git a/sql/expression/procedurereference.go b/sql/expression/procedurereference.go index 94b9bbb718..3645cd1240 100644 --- a/sql/expression/procedurereference.go +++ b/sql/expression/procedurereference.go @@ -271,7 +271,11 @@ var _ sql.CollationCoercible = (*ProcedureParam)(nil) // NewProcedureParam creates a new ProcedureParam expression. func NewProcedureParam(name string, typ sql.Type) *ProcedureParam { - return &ProcedureParam{name: strings.ToLower(name), typ: typ} + return &ProcedureParam{ + name: strings.ToLower(name), + typ: typ, + pRef: NewProcedureReference(), + } } // Children implements the sql.Expression interface. diff --git a/sql/plan/declare_variables.go b/sql/plan/declare_variables.go index 5fde5ccbf6..5f41dda4d1 100644 --- a/sql/plan/declare_variables.go +++ b/sql/plan/declare_variables.go @@ -40,6 +40,7 @@ func NewDeclareVariables(names []string, typ sql.Type, defaultVal *sql.ColumnDef Names: names, Type: typ, DefaultVal: defaultVal, + Pref: expression.NewProcedureReference(), } } From e9337fff570d91e8f28058ef71fa90ddabe0e674 Mon Sep 17 00:00:00 2001 From: jycor Date: Tue, 9 Apr 2024 21:32:10 +0000 Subject: [PATCH 2/5] [ga-format-pr] Run ./format_repo.sh to fix formatting --- enginetest/queries/trigger_queries.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index 15a28faad6..d9c1209293 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -2214,28 +2214,28 @@ INSERT INTO t0 (v1, v2) VALUES (i, s); END;`, }, Assertions: []ScriptTestAssertion{ { - Skip: true, - Query: "insert into t values (1);", + Skip: true, + Query: "insert into t values (1);", Expected: []sql.Row{ {types.NewOkResult(1)}, }, }, { - Skip: true, - Query: "insert into t values (2);", + Skip: true, + Query: "insert into t values (2);", Expected: []sql.Row{ {types.NewOkResult(1)}, }, }, { - Skip: true, + Skip: true, Query: "select * from t;", Expected: []sql.Row{ {10}, }, }, { - Skip: true, + Skip: true, Query: "select * from t;", Expected: []sql.Row{ {20}, @@ -2243,7 +2243,6 @@ INSERT INTO t0 (v1, v2) VALUES (i, s); END;`, }, }, }, - } // RollbackTriggerTests are trigger tests that require rollback logic to work correctly From 13cd8aeaf727f8675266f889758e6725efdef9a0 Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 9 Apr 2024 17:01:53 -0700 Subject: [PATCH 3/5] better panic --- enginetest/queries/trigger_queries.go | 32 +++++---------------------- sql/expression/procedurereference.go | 5 ++++- sql/plan/declare_variables.go | 1 - sql/planbuilder/create_ddl.go | 10 +++++++++ 4 files changed, 19 insertions(+), 29 deletions(-) diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index d9c1209293..b795617242 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -2206,40 +2206,18 @@ INSERT INTO t0 (v1, v2) VALUES (i, s); END;`, }, { - Name: "Trigger with BLOCK and DECLARE", + Name: "delete me", SetUpScript: []string{ "create table t (i int primary key);", - "create trigger trig before insert on t for each row begin declare x int; select new.i + 10 into x; set new.i = x; end;", - //"create trigger trig before insert on t for each row begin declare x int; set x = new.i * 10; set new.i = x; end;", }, Assertions: []ScriptTestAssertion{ { - Skip: true, - Query: "insert into t values (1);", - Expected: []sql.Row{ - {types.NewOkResult(1)}, - }, + Query: "create trigger trig1 before insert on t for each row begin declare x int; select new.i + 10 into x; set new.i = x; end;", + ExpectedErr: sql.ErrUnsupportedFeature, }, { - Skip: true, - Query: "insert into t values (2);", - Expected: []sql.Row{ - {types.NewOkResult(1)}, - }, - }, - { - Skip: true, - Query: "select * from t;", - Expected: []sql.Row{ - {10}, - }, - }, - { - Skip: true, - Query: "select * from t;", - Expected: []sql.Row{ - {20}, - }, + Query: "create trigger trig2 before insert on t for each row begin declare x int; set x = new.i * 10; set new.i = x; end;", + ExpectedErr: sql.ErrUnsupportedFeature, }, }, }, diff --git a/sql/expression/procedurereference.go b/sql/expression/procedurereference.go index 3645cd1240..7a7c47559f 100644 --- a/sql/expression/procedurereference.go +++ b/sql/expression/procedurereference.go @@ -60,6 +60,9 @@ type ProcedureReferencable interface { // InitializeVariable sets the initial value for the variable. func (ppr *ProcedureReference) InitializeVariable(name string, sqlType sql.Type, val interface{}) error { + if ppr == nil || ppr.InnermostScope == nil { + return fmt.Errorf("cannot initialize variable `%s` in an empty procedure reference", name) + } convertedVal, _, err := sqlType.Convert(val) if err != nil { return err @@ -274,7 +277,7 @@ func NewProcedureParam(name string, typ sql.Type) *ProcedureParam { return &ProcedureParam{ name: strings.ToLower(name), typ: typ, - pRef: NewProcedureReference(), + //pRef: NewProcedureReference(), } } diff --git a/sql/plan/declare_variables.go b/sql/plan/declare_variables.go index 5f41dda4d1..5fde5ccbf6 100644 --- a/sql/plan/declare_variables.go +++ b/sql/plan/declare_variables.go @@ -40,7 +40,6 @@ func NewDeclareVariables(names []string, typ sql.Type, defaultVal *sql.ColumnDef Names: names, Type: typ, DefaultVal: defaultVal, - Pref: expression.NewProcedureReference(), } } diff --git a/sql/planbuilder/create_ddl.go b/sql/planbuilder/create_ddl.go index ee9b4a8d48..ece33d9d38 100644 --- a/sql/planbuilder/create_ddl.go +++ b/sql/planbuilder/create_ddl.go @@ -108,6 +108,16 @@ func (b *Builder) buildCreateTrigger(inScope *scope, query string, c *ast.DDL) ( } } + // TODO: https://github.com/dolthub/dolt/issues/7720 + if block, isBEBlock := bodyScope.node.(*plan.BeginEndBlock); isBEBlock { + for _, child := range block.Children() { + if _, ok := child.(*plan.DeclareVariables); ok { + err := sql.ErrUnsupportedFeature.New("DECLARE in BEGIN END block") + b.handleErr(err) + } + } + } + outScope.node = plan.NewCreateTrigger( db, c.TriggerSpec.TrigName.Name.String(), From 9316e1cecc7445dfdcde6ef1ea61d98f2fe8bf87 Mon Sep 17 00:00:00 2001 From: jycor Date: Wed, 10 Apr 2024 00:03:05 +0000 Subject: [PATCH 4/5] [ga-format-pr] Run ./format_repo.sh to fix formatting --- enginetest/queries/trigger_queries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index b795617242..505c204d41 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -2212,11 +2212,11 @@ INSERT INTO t0 (v1, v2) VALUES (i, s); END;`, }, Assertions: []ScriptTestAssertion{ { - Query: "create trigger trig1 before insert on t for each row begin declare x int; select new.i + 10 into x; set new.i = x; end;", + Query: "create trigger trig1 before insert on t for each row begin declare x int; select new.i + 10 into x; set new.i = x; end;", ExpectedErr: sql.ErrUnsupportedFeature, }, { - Query: "create trigger trig2 before insert on t for each row begin declare x int; set x = new.i * 10; set new.i = x; end;", + Query: "create trigger trig2 before insert on t for each row begin declare x int; set x = new.i * 10; set new.i = x; end;", ExpectedErr: sql.ErrUnsupportedFeature, }, }, From a595bbe5748a52a6dc08670161d96e796b97a60f Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 10 Apr 2024 00:29:36 -0700 Subject: [PATCH 5/5] remove comment --- sql/expression/procedurereference.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/expression/procedurereference.go b/sql/expression/procedurereference.go index 7a7c47559f..496a5b025c 100644 --- a/sql/expression/procedurereference.go +++ b/sql/expression/procedurereference.go @@ -277,7 +277,6 @@ func NewProcedureParam(name string, typ sql.Type) *ProcedureParam { return &ProcedureParam{ name: strings.ToLower(name), typ: typ, - //pRef: NewProcedureReference(), } }