Skip to content

Commit 70a452a

Browse files
author
James Cor
committed
return error for ddl in create proc statements
1 parent f5a5bce commit 70a452a

File tree

5 files changed

+131
-61
lines changed

5 files changed

+131
-61
lines changed

enginetest/queries/procedure_queries.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2823,6 +2823,64 @@ var ProcedureCreateInSubroutineTests = []ScriptTest{
28232823
},
28242824
},
28252825
},
2826+
{
2827+
Name: "procedure must not contain CREATE TABLE",
2828+
Assertions: []ScriptTestAssertion{
2829+
{
2830+
Query: "create procedure p() create table t (pk int);",
2831+
ExpectedErrStr: "creating tables in stored procedures is currently unsupported and will be added in a future release",
2832+
},
2833+
{
2834+
Query: "create procedure p() begin create table t (pk int); end;",
2835+
ExpectedErrStr: "creating tables in stored procedures is currently unsupported and will be added in a future release",
2836+
},
2837+
},
2838+
},
2839+
{
2840+
Name: "procedure must not contain CREATE TRIGGER",
2841+
SetUpScript: []string{
2842+
"create table t (i int);",
2843+
},
2844+
Assertions: []ScriptTestAssertion{
2845+
{
2846+
Query: "create procedure p() create trigger trig before insert on t for each row begin select 1; end;",
2847+
ExpectedErrStr: "creating triggers in stored procedures is currently unsupported and will be added in a future release",
2848+
},
2849+
{
2850+
Query: "create procedure p() begin create trigger trig before insert on t for each row begin select 1; end; end;",
2851+
ExpectedErrStr: "creating triggers in stored procedures is currently unsupported and will be added in a future release",
2852+
},
2853+
},
2854+
},
2855+
{
2856+
Name: "procedure must not contain CREATE DB",
2857+
SetUpScript: []string{},
2858+
Assertions: []ScriptTestAssertion{
2859+
{
2860+
Query: "create procedure p() create database procdb;",
2861+
ExpectedErrStr: "creating databases in stored procedures is currently unsupported and will be added in a future release",
2862+
},
2863+
{
2864+
Query: "create procedure p() begin create database procdb; end;",
2865+
ExpectedErrStr: "creating databases in stored procedures is currently unsupported and will be added in a future release",
2866+
},
2867+
},
2868+
},
2869+
{
2870+
Name: "procedure must not contain CREATE VIEW",
2871+
SetUpScript: []string{},
2872+
Assertions: []ScriptTestAssertion{
2873+
{
2874+
Query: "create procedure p() create view v as select 1;",
2875+
ExpectedErrStr: "creating views in stored procedures is currently unsupported and will be added in a future release",
2876+
},
2877+
{
2878+
Query: "create procedure p() begin create view v as select 1; end;",
2879+
ExpectedErrStr: "creating views in stored procedures is currently unsupported and will be added in a future release",
2880+
},
2881+
},
2882+
},
2883+
28262884
}
28272885

28282886
var NoDbProcedureTests = []ScriptTestAssertion{

sql/analyzer/stored_procedures.go

Lines changed: 7 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,14 @@
1515
package analyzer
1616

1717
import (
18-
"fmt"
19-
"slices"
20-
"strings"
2118

22-
"gopkg.in/src-d/go-errors.v1"
23-
24-
"github.com/dolthub/go-mysql-server/sql"
25-
"github.com/dolthub/go-mysql-server/sql/expression"
26-
"github.com/dolthub/go-mysql-server/sql/plan"
27-
"github.com/dolthub/go-mysql-server/sql/planbuilder"
28-
"github.com/dolthub/go-mysql-server/sql/transform"
19+
"fmt"
20+
"github.com/dolthub/go-mysql-server/sql"
21+
"github.com/dolthub/go-mysql-server/sql/expression"
22+
"github.com/dolthub/go-mysql-server/sql/plan"
23+
"github.com/dolthub/go-mysql-server/sql/planbuilder"
24+
"github.com/dolthub/go-mysql-server/sql/transform"
25+
"slices"
2926
)
3027

3128
// loadStoredProcedures loads non-built-in stored procedures for all databases on relevant calls.
@@ -184,56 +181,6 @@ func validateCreateProcedure(ctx *sql.Context, a *Analyzer, node sql.Node, scope
184181
func validateStoredProcedure(_ *sql.Context, proc *plan.Procedure) error {
185182
// For now, we don't support creating any of the following within stored procedures.
186183
// These will be removed in the future, but cause issues with the current execution plan.
187-
var err error
188-
spUnsupportedErr := errors.NewKind("creating %s in stored procedures is currently unsupported " +
189-
"and will be added in a future release")
190-
transform.Inspect(proc, func(n sql.Node) bool {
191-
switch n.(type) {
192-
case *plan.CreateTable:
193-
err = spUnsupportedErr.New("tables")
194-
case *plan.CreateTrigger:
195-
err = spUnsupportedErr.New("triggers")
196-
case *plan.CreateProcedure:
197-
err = spUnsupportedErr.New("procedures")
198-
case *plan.CreateDB:
199-
err = spUnsupportedErr.New("databases")
200-
case *plan.CreateForeignKey:
201-
err = spUnsupportedErr.New("foreign keys")
202-
case *plan.CreateIndex:
203-
err = spUnsupportedErr.New("indexes")
204-
case *plan.CreateView:
205-
err = spUnsupportedErr.New("views")
206-
default:
207-
return true
208-
}
209-
return false
210-
})
211-
if err != nil {
212-
return err
213-
}
214-
215-
transform.Inspect(proc, func(n sql.Node) bool {
216-
switch n := n.(type) {
217-
case *plan.Call:
218-
if proc.Name == strings.ToLower(n.Name) {
219-
err = sql.ErrProcedureRecursiveCall.New(proc.Name)
220-
}
221-
case *plan.LockTables: // Blocked in vitess, but this is for safety
222-
err = sql.ErrProcedureInvalidBodyStatement.New("LOCK TABLES")
223-
case *plan.UnlockTables: // Blocked in vitess, but this is for safety
224-
err = sql.ErrProcedureInvalidBodyStatement.New("UNLOCK TABLES")
225-
case *plan.Use: // Blocked in vitess, but this is for safety
226-
err = sql.ErrProcedureInvalidBodyStatement.New("USE")
227-
case *plan.LoadData:
228-
err = sql.ErrProcedureInvalidBodyStatement.New("LOAD DATA")
229-
default:
230-
return true
231-
}
232-
return false
233-
})
234-
if err != nil {
235-
return err
236-
}
237184

238185
return nil
239186
}

sql/planbuilder/create_ddl.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ package planbuilder
1616

1717
import (
1818
"fmt"
19-
"strings"
19+
"github.com/dolthub/go-mysql-server/sql/transform"
20+
"strings"
2021
"time"
2122
"unicode"
2223

@@ -132,6 +133,9 @@ func getCurrentUserForDefiner(ctx *sql.Context, definer string) string {
132133
}
133134

134135
func (b *Builder) buildCreateProcedure(inScope *scope, subQuery string, fullQuery string, c *ast.DDL) (outScope *scope) {
136+
b.qFlags.Set(sql.QFlagCreateProcedure)
137+
defer func() {b.qFlags.Unset(sql.QFlagCreateProcedure)}()
138+
135139
var params []plan.ProcedureParam
136140
for _, param := range c.ProcedureSpec.Params {
137141
var direction plan.ProcedureParamDirection
@@ -200,6 +204,20 @@ func (b *Builder) buildCreateProcedure(inScope *scope, subQuery string, fullQuer
200204
bodyStr := strings.TrimSpace(fullQuery[c.SubStatementPositionStart:c.SubStatementPositionEnd])
201205

202206
bodyScope := b.buildSubquery(inScope, c.ProcedureSpec.Body, bodyStr, fullQuery)
207+
b.validateStoredProcedure(bodyScope.node)
208+
209+
// Check for recursive calls to same procedure
210+
transform.Inspect(bodyScope.node, func(node sql.Node) (bool) {
211+
switch n := node.(type) {
212+
case *plan.Call:
213+
if strings.EqualFold(procName, n.Name) {
214+
b.handleErr(sql.ErrProcedureRecursiveCall.New(procName))
215+
}
216+
return false
217+
default:
218+
return true
219+
}
220+
})
203221

204222
var db sql.Database = nil
205223
dbName := c.ProcedureSpec.ProcName.Qualifier.String()

sql/planbuilder/proc.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package planbuilder
1616

1717
import (
1818
"fmt"
19+
"gopkg.in/src-d/go-errors.v1"
1920
"strconv"
2021
"strings"
2122

@@ -24,6 +25,7 @@ import (
2425
"github.com/dolthub/go-mysql-server/sql"
2526
"github.com/dolthub/go-mysql-server/sql/expression"
2627
"github.com/dolthub/go-mysql-server/sql/plan"
28+
"github.com/dolthub/go-mysql-server/sql/transform"
2729
"github.com/dolthub/go-mysql-server/sql/types"
2830
)
2931

@@ -404,11 +406,55 @@ func (b *Builder) buildBlock(inScope *scope, parserStatements ast.Statements, fu
404406
}
405407
}
406408
stmtScope := b.buildSubquery(inScope, s, ast.String(s), fullQuery)
409+
if b.qFlags.IsSet(sql.QFlagCreateProcedure) {
410+
b.validateStoredProcedure(stmtScope.node)
411+
}
407412
statements = append(statements, stmtScope.node)
408413
}
414+
409415
return plan.NewBlock(statements)
410416
}
411417

418+
func (b *Builder) validateStoredProcedure(node sql.Node) {
419+
// For now, we don't support creating any of the following within stored procedures.
420+
// These will be removed in the future, but cause issues with the current execution plan.
421+
var err error
422+
spUnsupportedErr := errors.NewKind("creating %s in stored procedures is currently unsupported " +
423+
"and will be added in a future release")
424+
transform.Inspect(node, func(n sql.Node) bool {
425+
switch n.(type) {
426+
case *plan.CreateTable:
427+
err = spUnsupportedErr.New("tables")
428+
case *plan.CreateTrigger:
429+
err = spUnsupportedErr.New("triggers")
430+
case *plan.CreateProcedure:
431+
err = spUnsupportedErr.New("procedures")
432+
case *plan.CreateDB:
433+
err = spUnsupportedErr.New("databases")
434+
case *plan.CreateForeignKey:
435+
err = spUnsupportedErr.New("foreign keys")
436+
case *plan.CreateIndex:
437+
err = spUnsupportedErr.New("indexes")
438+
case *plan.CreateView:
439+
err = spUnsupportedErr.New("views")
440+
case *plan.LockTables: // Blocked in vitess, but this is for safety
441+
err = sql.ErrProcedureInvalidBodyStatement.New("LOCK TABLES")
442+
case *plan.UnlockTables: // Blocked in vitess, but this is for safety
443+
err = sql.ErrProcedureInvalidBodyStatement.New("UNLOCK TABLES")
444+
case *plan.Use: // Blocked in vitess, but this is for safety
445+
err = sql.ErrProcedureInvalidBodyStatement.New("USE")
446+
case *plan.LoadData:
447+
err = sql.ErrProcedureInvalidBodyStatement.New("LOAD DATA")
448+
default:
449+
return true
450+
}
451+
return false
452+
})
453+
if err != nil {
454+
b.handleErr(err)
455+
}
456+
}
457+
412458
func (b *Builder) buildFetchCursor(inScope *scope, fetchCursor *ast.FetchCursor) (outScope *scope) {
413459
if !inScope.proc.HasCursor(fetchCursor.Name) {
414460
err := sql.ErrCursorNotFound.New(fetchCursor.Name)

sql/query_flags.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const (
5151
// QFlagUndeferrableExprs indicates that the query has expressions that cannot be deferred
5252
QFlagUndeferrableExprs
5353
QFlagTrigger
54+
QFlagCreateProcedure
5455
)
5556

5657
type QueryFlags struct {

0 commit comments

Comments
 (0)