Skip to content

Commit

Permalink
Stored procedures can use params as LIMIT,OFFSET (#2315)
Browse files Browse the repository at this point in the history
* Stored procedures can use params as LIMIT,OFFSET

* error tests and fix proc param types
  • Loading branch information
max-hoffman authored Feb 6, 2024
1 parent d5876b4 commit 33cd2ef
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 34 deletions.
38 changes: 34 additions & 4 deletions enginetest/queries/procedure_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,15 +715,15 @@ END;`,
{
Query: "CALL p1(3, 4)",
Expected: []sql.Row{
{"4", "6"},
{"3", "4"},
{4, 6},
{3, 4},
},
},
{
Query: "CALL p2(5, 6)",
Expected: []sql.Row{
{"6", "8"},
{"5", "6"},
{6, 8},
{5, 6},
},
},
},
Expand Down Expand Up @@ -1153,6 +1153,36 @@ END;`,
},
},
},
{
Name: "issue 7458: proc params as limit values",
SetUpScript: []string{
"create table t (i int primary key);",
"insert into t values (0), (1), (2), (3)",
"CREATE PROCEDURE limited(the_limit int, the_offset bigint) SELECT * FROM t LIMIT the_limit OFFSET the_offset",
},
Assertions: []ScriptTestAssertion{
{
Query: "call limited(1,0)",
Expected: []sql.Row{{0}},
},
{
Query: "call limited(2,0)",
Expected: []sql.Row{{0}, {1}},
},
{
Query: "call limited(2,2)",
Expected: []sql.Row{{2}, {3}},
},
{
Query: "CREATE PROCEDURE limited_inv(the_limit CHAR(3), the_offset INT) SELECT * FROM t LIMIT the_limit OFFSET the_offset",
ExpectedErrStr: "the variable 'the_limit' has a non-integer based type: char(3) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_bin",
},
{
Query: "CREATE PROCEDURE limited_inv(the_limit float, the_offset INT) SELECT * FROM t LIMIT the_limit OFFSET the_offset",
ExpectedErrStr: "the variable 'the_limit' has a non-integer based type: float",
},
},
},
{
Name: "FETCH captures state at OPEN",
SetUpScript: []string{
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e
github.com/dolthub/jsonpath v0.0.2-0.20240201003050-392940944c15
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
github.com/dolthub/vitess v0.0.0-20240129233432-aec9daef6af7
github.com/dolthub/vitess v0.0.0-20240205203605-9e6c6d650813
github.com/go-kit/kit v0.10.0
github.com/go-sql-driver/mysql v1.7.2-0.20231213112541-0004702b931d
github.com/gocraft/dbr/v2 v2.7.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ github.com/dolthub/jsonpath v0.0.2-0.20240201003050-392940944c15 h1:sfTETOpsrNJP
github.com/dolthub/jsonpath v0.0.2-0.20240201003050-392940944c15/go.mod h1:2/2zjLQ/JOOSbbSboojeg+cAwcRV0fDLzIiWch/lhqI=
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9XGFa6q5Ap4Z/OhNkAMBaK5YeuEzwJt+NZdhiE=
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
github.com/dolthub/vitess v0.0.0-20240129233432-aec9daef6af7 h1:AhmDCMtoEh2PwYsfblCaWIVvpHgDmWhz1YNNwl67vm4=
github.com/dolthub/vitess v0.0.0-20240129233432-aec9daef6af7/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw=
github.com/dolthub/vitess v0.0.0-20240205203605-9e6c6d650813 h1:tGwsoLAMFQ+7FDEyIWOIJ1Vc/nptbFi0Fh7SQahB8ro=
github.com/dolthub/vitess v0.0.0-20240205203605-9e6c6d650813/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw=
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs=
github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU=
Expand Down
4 changes: 2 additions & 2 deletions sql/analyzer/resolve_external_stored_procedures.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,15 @@ func resolveExternalStoredProcedure(_ *sql.Context, externalProcedure sql.Extern
Type: sqlType,
Variadic: paramIsVariadic,
}
paramReferences[i] = expression.NewProcedureParam(paramName)
paramReferences[i] = expression.NewProcedureParam(paramName, sqlType)
} else if sqlType, ok = externalStoredProcedurePointerTypes[funcParamType]; ok {
paramDefinitions[i] = plan.ProcedureParam{
Direction: plan.ProcedureParamDirection_Inout,
Name: paramName,
Type: sqlType,
Variadic: paramIsVariadic,
}
paramReferences[i] = expression.NewProcedureParam(paramName)
paramReferences[i] = expression.NewProcedureParam(paramName, sqlType)
} else {
return nil, sql.ErrExternalProcedureInvalidParamType.New(funcParamType.String())
}
Expand Down
4 changes: 2 additions & 2 deletions sql/analyzer/validation_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func validateLimitAndOffset(ctx *sql.Context, a *Analyzer, n sql.Node, scope *pl
err = sql.ErrInvalidSyntax.New("negative limit")
return false
}
case *expression.BindVar:
case *expression.BindVar, *expression.ProcedureParam:
return true
default:
err = sql.ErrInvalidType.New(e.Type().String())
Expand All @@ -81,7 +81,7 @@ func validateLimitAndOffset(ctx *sql.Context, a *Analyzer, n sql.Node, scope *pl
err = sql.ErrInvalidSyntax.New("negative offset")
return false
}
case *expression.BindVar:
case *expression.BindVar, *expression.ProcedureParam:
return true
default:
err = sql.ErrInvalidType.New(e.Type().String())
Expand Down
7 changes: 4 additions & 3 deletions sql/expression/procedurereference.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,16 @@ func NewProcedureReference() *ProcedureReference {
type ProcedureParam struct {
name string
pRef *ProcedureReference
typ sql.Type
hasBeenSet bool
}

var _ sql.Expression = (*ProcedureParam)(nil)
var _ sql.CollationCoercible = (*ProcedureParam)(nil)

// NewProcedureParam creates a new ProcedureParam expression.
func NewProcedureParam(name string) *ProcedureParam {
return &ProcedureParam{name: strings.ToLower(name)}
func NewProcedureParam(name string, typ sql.Type) *ProcedureParam {
return &ProcedureParam{name: strings.ToLower(name), typ: typ}
}

// Children implements the sql.Expression interface.
Expand All @@ -287,7 +288,7 @@ func (*ProcedureParam) IsNullable() bool {

// Type implements the sql.Expression interface.
func (pp *ProcedureParam) Type() sql.Type {
return pp.pRef.GetVariableType(pp.name)
return pp.typ
}

// CollationCoercibility implements the sql.CollationCoercible interface.
Expand Down
2 changes: 1 addition & 1 deletion sql/plan/procedure.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func (p *Procedure) ExtendVariadic(ctx *sql.Context, length int) *Procedure {
Type: variadicParam.Type,
Variadic: variadicParam.Variadic,
}
newParams[i] = expression.NewProcedureParam(paramName)
newParams[i] = expression.NewProcedureParam(paramName, variadicParam.Type)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion sql/planbuilder/create_ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (b *Builder) buildCreateProcedure(inScope *scope, query string, c *ast.DDL)
// populate inScope with the procedure parameters. this will be
// subject maybe a bug where an inner procedure has access to
// outer procedure parameters.
inScope.proc.AddVar(expression.NewProcedureParam(strings.ToLower(p.Name)))
inScope.proc.AddVar(expression.NewProcedureParam(strings.ToLower(p.Name), p.Type))
}
bodyStr := strings.TrimSpace(query[c.SubStatementPositionStart:c.SubStatementPositionEnd])

Expand Down
2 changes: 1 addition & 1 deletion sql/planbuilder/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func (b *Builder) buildDeclareVariables(inScope *scope, d *ast.Declare) (outScop
for i, variable := range dVars.Names {
varName := strings.ToLower(variable.String())
names[i] = varName
param := expression.NewProcedureParam(varName)
param := expression.NewProcedureParam(varName, typ)
inScope.proc.AddVar(param)
inScope.newColumn(scopeColumn{col: varName, typ: typ, scalar: param})
}
Expand Down
60 changes: 43 additions & 17 deletions sql/planbuilder/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,49 @@ func (b *Builder) buildSelect(inScope *scope, s *ast.Select) (outScope *scope) {

func (b *Builder) buildLimit(inScope *scope, limit *ast.Limit) sql.Expression {
if limit != nil {
l := b.buildScalar(inScope, limit.Rowcount)
return b.buildLimitVal(inScope, limit.Rowcount)
}
return nil
}

func (b *Builder) buildOffset(inScope *scope, limit *ast.Limit) sql.Expression {
if limit != nil && limit.Offset != nil {
e := b.buildLimitVal(inScope, limit.Offset)
if lit, ok := e.(*expression.Literal); ok {
// Check if offset starts at 0, if so, we can just remove the offset node.
// Only cast to int8, as a larger int type just means a non-zero offset.
if val, err := lit.Eval(b.ctx, nil); err == nil {
if v, ok := val.(int64); ok && v == 0 {
return nil
}
}
}
return e
}
return nil
}

// buildLimitVal resolves a literal numeric type or a numeric
// prodecure parameter
func (b *Builder) buildLimitVal(inScope *scope, e ast.Expr) sql.Expression {
switch e := e.(type) {
case *ast.ColName:
if inScope.procActive() {
if col, ok := inScope.proc.GetVar(e.String()); ok {
// proc param is OK
if pp, ok := col.scalarGf().(*expression.ProcedureParam); ok {
if !pp.Type().Promote().Equals(types.Int64) {
err := fmt.Errorf("the variable '%s' has a non-integer based type: %s", pp.Name(), pp.Type().String())
b.handleErr(err)
}
return pp
}
}
}
err := fmt.Errorf("limit expression expected to be numeric or prodecure parameter, found invalid column: %s", e.String())
b.handleErr(err)
default:
l := b.buildScalar(inScope, e)
return b.typeCoerceLiteral(l)
}
return nil
Expand All @@ -150,22 +192,6 @@ func (b *Builder) typeCoerceLiteral(e sql.Expression) sql.Expression {
return nil
}

func (b *Builder) buildOffset(inScope *scope, limit *ast.Limit) sql.Expression {
if limit != nil && limit.Offset != nil {
rowCount := b.buildScalar(inScope, limit.Offset)
rowCount = b.typeCoerceLiteral(rowCount)
// Check if offset starts at 0, if so, we can just remove the offset node.
// Only cast to int8, as a larger int type just means a non-zero offset.
if val, err := rowCount.Eval(b.ctx, nil); err == nil {
if v, ok := val.(int64); ok && v == 0 {
return nil
}
}
return rowCount
}
return nil
}

// buildDistinct creates a new plan.Distinct node if the query has a DISTINCT option.
// If the query has both DISTINCT and ALL, an error is returned.
func (b *Builder) buildDistinct(inScope *scope, distinct bool) {
Expand Down

0 comments on commit 33cd2ef

Please sign in to comment.