fixed external stored procedures, moving onto other procedure errors

Author: James Cor

enginetest/queries/procedure_queries.go

@@ -2441,6 +2441,20 @@ var ProcedureCallTests = []ScriptTest{
 			},
 		},
 	},
+	{
+		Name: "creating invalid procedure doesn't error until it is called",
+		Assertions: []ScriptTestAssertion{
+			{
+				Query: `CREATE PROCEDURE proc1 (OUT out_count INT) READS SQL DATA SELECT COUNT(*) FROM mytable WHERE i = 1 AND s = 'first row' AND func1(i);`,
+				Expected: []sql.Row{{types.NewOkResult(0)}},
+			},
+			{
+				Query:       "CALL proc1(@out_count);",
+				ExpectedErr: sql.ErrFunctionNotFound,
+			},
+		},
+
+	},
 }
 
 var ProcedureDropTests = []ScriptTest{

enginetest/queries/queries.go

@@ -10956,11 +10956,6 @@ var ErrorQueries = []QueryErrorTest{
 		Query:       `SELECT * FROM datetime_table where datetime_col >= 'not a valid datetime'`,
 		ExpectedErr: types.ErrConvertingToTime,
 	},
-	// this query was panicing, but should be allowed and should return error when this query is called
-	{
-		Query:       `CREATE PROCEDURE proc1 (OUT out_count INT) READS SQL DATA SELECT COUNT(*) FROM mytable WHERE i = 1 AND s = 'first row' AND func1(i);`,
-		ExpectedErr: sql.ErrFunctionNotFound,
-	},
 	{
 		Query:       "CREATE TABLE table_test (id int PRIMARY KEY, c float DEFAULT rand())",
 		ExpectedErr: sql.ErrSyntaxError,

sql/analyzer/stored_procedures.go

@@ -27,6 +27,7 @@ import (
 
 // loadStoredProcedures loads non-built-in stored procedures for all databases on relevant calls.
 func loadStoredProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, sel RuleSelector) (*plan.Scope, error) {
+	// TODO: possible that we can just delete this entire rule
 	if scope.ProceduresPopulating() {
 		return scope, nil
 	}
@@ -51,7 +52,7 @@ func loadStoredProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan
 			return nil, err
 		}
 		for _, procedure := range procedures {
-			proc := planbuilder.BuildProcedureHelper(ctx, a.Catalog, database, nil, procedure)
+			proc := planbuilder.BuildProcedureHelper(ctx, a.Catalog, nil, database, nil, procedure)
 			err = scope.Procedures.Register(database.Name(), proc)
 			if err != nil {
 				return nil, err

sql/planbuilder/proc.go

@@ -226,7 +226,7 @@ func (b *Builder) buildIfConditional(inScope *scope, n ast.IfStatementCondition,
 	return outScope
 }
 
-func BuildProcedureHelper(ctx *sql.Context, cat sql.Catalog, db sql.Database, asOf sql.Expression, proc sql.StoredProcedureDetails) *plan.Procedure {
+func BuildProcedureHelper(ctx *sql.Context, cat sql.Catalog, inScope *scope, db sql.Database, asOf sql.Expression, proc sql.StoredProcedureDetails) *plan.Procedure {
 	// TODO: new builder necessary?
 	b := New(ctx, cat, nil, nil)
 	b.DisableAuth()
@@ -241,14 +241,26 @@ func BuildProcedureHelper(ctx *sql.Context, cat sql.Catalog, db sql.Database, as
 	b.ProcCtx().DbName = db.Name()
 	stmt, _, _, _ := b.parser.ParseWithOptions(b.ctx, proc.CreateStatement, ';', false, b.parserOpts)
 	procStmt := stmt.(*ast.DDL)
-	bodyStr := strings.TrimSpace(proc.CreateStatement[procStmt.SubStatementPositionStart:procStmt.SubStatementPositionEnd])
-	bodyScope := b.buildSubquery(nil, procStmt.ProcedureSpec.Body, bodyStr, proc.CreateStatement) // TODO: scope?
-
-	// TODO: validate
 
 	procParams := b.buildProcedureParams(procStmt.ProcedureSpec.Params)
 	characteristics, securityType, comment := b.buildProcedureCharacteristics(procStmt.ProcedureSpec.Characteristics)
 
+	// populate inScope with the procedure parameters. this will be
+	// subject maybe a bug where an inner procedure has access to
+	// outer procedure parameters.
+	if inScope == nil {
+		inScope = b.newScope()
+	}
+	inScope.initProc()
+	for _, p := range procParams {
+		inScope.proc.AddVar(expression.NewProcedureParam(strings.ToLower(p.Name), p.Type))
+	}
+
+	bodyStr := strings.TrimSpace(proc.CreateStatement[procStmt.SubStatementPositionStart:procStmt.SubStatementPositionEnd])
+	bodyScope := b.buildSubquery(inScope, procStmt.ProcedureSpec.Body, bodyStr, proc.CreateStatement)
+
+	// TODO: validate
+
 	return plan.NewProcedure(
 		proc.Name,
 		procStmt.ProcedureSpec.Definer,
@@ -303,7 +315,7 @@ func (b *Builder) buildCall(inScope *scope, c *ast.Call) (outScope *scope) {
 		procDetails, ok, err = spdb.GetStoredProcedure(b.ctx, procName)
 		if err == nil {
 			if ok {
-				proc = BuildProcedureHelper(b.ctx, b.cat, db, asOf, procDetails)
+				proc = BuildProcedureHelper(b.ctx, b.cat, inScope, db, asOf, procDetails)
 			} else {
 				err = sql.ErrStoredProcedureDoesNotExist.New(procName)
 			}
@@ -315,14 +327,6 @@ func (b *Builder) buildCall(inScope *scope, c *ast.Call) (outScope *scope) {
 		b.handleErr(err)
 	}
 
-	// populate inScope with the procedure parameters. this will be
-	// subject maybe a bug where an inner procedure has access to
-	// outer procedure parameters.
-	inScope.initProc()
-	for _, p := range proc.Params {
-		inScope.proc.AddVar(expression.NewProcedureParam(strings.ToLower(p.Name), p.Type))
-	}
-
 	params := make([]sql.Expression, len(c.Params))
 	for i, param := range c.Params {
 		expr := b.buildScalar(inScope, param)