Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stored procedures can use params as LIMIT,OFFSET #2315

Merged
merged 2 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading