Skip to content

Commit a57625e

Browse files
jenniferspjennifersp
committed
update for feedback
1 parent f19589b commit a57625e

File tree

10 files changed

+59
-41
lines changed

10 files changed

+59
-41
lines changed

‎server/analyzer/init.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ func Init() {
6363
analyzer.OnceBeforeDefault...)
6464

6565
analyzer.AlwaysBeforeDefault = append(analyzer.AlwaysBeforeDefault,
66+
// ResolveType rule must run in this batch in addition to OnceBeforeDefault batch
67+
// because of custom batch set optimization in GMS skipping OnceBeforeDefault batch for some nodes.
6668
analyzer.Rule{Id: ruleId_ResolveType, Apply: ResolveType},
6769
analyzer.Rule{Id: ruleId_TypeSanitizer, Apply: TypeSanitizer},
6870
analyzer.Rule{Id: ruleId_ResolveValuesTypes, Apply: ResolveValuesTypes},

‎server/ast/create_function.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func nodeCreateFunction(ctx *Context, node *tree.CreateFunction) (vitess.Stateme
5353
retType = createAnonymousCompositeType(node.RetType)
5454
}
5555

56-
params := make([]pgnodes.RoutineArg, len(node.Args))
56+
params := make([]pgnodes.RoutineParam, len(node.Args))
5757
var defaults []vitess.Expr
5858
for i, arg := range node.Args {
5959
// parameter name
@@ -123,7 +123,7 @@ func nodeCreateFunction(ctx *Context, node *tree.CreateFunction) (vitess.Stateme
123123
if ok {
124124
beginAtomic, ok := sqlBody.SqlBody.(*tree.BeginEndBlock)
125125
if !ok {
126-
return nil, errors.Errorf("Expected BEGIN ATOMIC in CREATE FUNCTION definition, got %T", sqlBody.SqlBody)
126+
return nil, errors.Errorf("Expected BEGIN in CREATE FUNCTION definition, got %T", sqlBody.SqlBody)
127127
}
128128
stmts := make([]parser.Statement, len(beginAtomic.Statements))
129129
for i, s := range beginAtomic.Statements {
@@ -205,8 +205,9 @@ func createAnonymousCompositeType(fieldTypes []tree.SimpleColumnDef) *pgtypes.Do
205205
return pgtypes.NewCompositeType(context.Background(), id.Null, id.NullType, typeId, attrs)
206206
}
207207

208-
// handleLanguageSQLAs handles parsing SQL definition strings in both CREATE FUNCTION and CREATE PROCEDURE.
209-
func handleLanguageSQLAs(definition string, params []pgnodes.RoutineArg) (string, []vitess.Statement, error) {
208+
// handleLanguageSQLAs handles parsing SQL definition strings in both CREATE FUNCTION and CREATE PROCEDURE
209+
// and returns converted the sql statements into vitess statements.
210+
func handleLanguageSQLAs(definition string, params []pgnodes.RoutineParam) (string, []vitess.Statement, error) {
210211
stmts, err := parser.Parse(definition)
211212
if err != nil {
212213
return "", nil, err
@@ -215,7 +216,9 @@ func handleLanguageSQLAs(definition string, params []pgnodes.RoutineArg) (string
215216
return convertSQLStmts(stmts, params)
216217
}
217218

218-
func convertSQLStmts(stmts []parser.Statement, params []pgnodes.RoutineArg) (string, []vitess.Statement, error) {
219+
// convertSQLStmts takes parser.Statements and routine parameters and
220+
// returns converted to string representation and vitess statements.
221+
func convertSQLStmts(stmts parser.Statements, params []pgnodes.RoutineParam) (string, []vitess.Statement, error) {
219222
paramMap := make(map[string]*framework.ParamTypAndValue, len(params))
220223
for i, param := range params {
221224
tv := &framework.ParamTypAndValue{

‎server/ast/create_procedure.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func nodeCreateProcedure(ctx *Context, node *tree.CreateProcedure) (vitess.State
3737
}
3838
// Grab the general information that we'll need to create the procedure
3939
tableName := node.Name.ToTableName()
40-
params := make([]pgnodes.RoutineArg, len(node.Args))
40+
params := make([]pgnodes.RoutineParam, len(node.Args))
4141
var defaults []vitess.Expr
4242
for i, arg := range node.Args {
4343
// parameter name

‎server/ast/drop_function.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,23 +37,23 @@ func nodeDropFunction(ctx *Context, node *tree.DropFunction) (vitess.Statement,
3737
return nil, fmt.Errorf("no function name specified for DROP FUNCTION")
3838
}
3939

40-
functions := make([]*pgnodes.RoutineWithArgs, len(node.Functions))
40+
functions := make([]*pgnodes.RoutineWithParams, len(node.Functions))
4141
for i, fn := range node.Functions {
42-
var args []pgnodes.RoutineArg
42+
var args []pgnodes.RoutineParam
4343
for _, a := range fn.Args {
4444
if a.Mode != tree.RoutineArgModeOut {
4545
_, dt, err := nodeResolvableTypeReference(ctx, a.Type, false)
4646
if err != nil {
4747
return nil, err
4848
}
49-
args = append(args, pgnodes.RoutineArg{
49+
args = append(args, pgnodes.RoutineParam{
5050
Name: a.Name.String(),
5151
Type: dt,
5252
})
5353
}
5454
}
5555
objName := fn.Name.ToTableName()
56-
functions[i] = &pgnodes.RoutineWithArgs{
56+
functions[i] = &pgnodes.RoutineWithParams{
5757
Args: args,
5858
SchemaName: objName.Schema(),
5959
RoutineName: objName.Object(),

‎server/ast/drop_procedure.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,23 +37,23 @@ func nodeDropProcedure(ctx *Context, node *tree.DropProcedure) (vitess.Statement
3737
return nil, fmt.Errorf("no function name specified for DROP PROCEDURE")
3838
}
3939

40-
procedures := make([]*pgnodes.RoutineWithArgs, len(node.Procedures))
40+
procedures := make([]*pgnodes.RoutineWithParams, len(node.Procedures))
4141
for i, fn := range node.Procedures {
42-
var args []pgnodes.RoutineArg
42+
var args []pgnodes.RoutineParam
4343
for _, a := range fn.Args {
4444
if a.Mode != tree.RoutineArgModeOut {
4545
_, dt, err := nodeResolvableTypeReference(ctx, a.Type, false)
4646
if err != nil {
4747
return nil, err
4848
}
49-
args = append(args, pgnodes.RoutineArg{
49+
args = append(args, pgnodes.RoutineParam{
5050
Name: a.Name.String(),
5151
Type: dt,
5252
})
5353
}
5454
}
5555
objName := fn.Name.ToTableName()
56-
procedures[i] = &pgnodes.RoutineWithArgs{
56+
procedures[i] = &pgnodes.RoutineWithParams{
5757
Args: args,
5858
SchemaName: objName.Schema(),
5959
RoutineName: objName.Object(),

‎server/node/create_function.go‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,8 @@ import (
3131
pgtypes "github.com/dolthub/doltgresql/server/types"
3232
)
3333

34-
// RoutineWithArgs represent a function or a procedure with schema name, routine name and its arguments.
35-
type RoutineWithArgs struct {
36-
SchemaName string
37-
RoutineName string
38-
Args []RoutineArg
39-
}
40-
41-
// RoutineArg represents a routine parameter with parameter name and parameter type.
42-
type RoutineArg struct {
34+
// RoutineParam represents a routine parameter with parameter name, type and default value if exists.
35+
type RoutineParam struct {
4336
Mode procedures.ParameterMode
4437
Name string
4538
Type *pgtypes.DoltgresType
@@ -53,7 +46,7 @@ type CreateFunction struct {
5346
SchemaName string
5447
Replace bool
5548
ReturnType *pgtypes.DoltgresType
56-
Parameters []RoutineArg
49+
Parameters []RoutineParam
5750
Strict bool
5851
Statements []plpgsql.InterpreterOperation
5952
ExtensionName string
@@ -73,7 +66,7 @@ func NewCreateFunction(
7366
schemaName string,
7467
replace bool,
7568
retType *pgtypes.DoltgresType,
76-
params []RoutineArg,
69+
params []RoutineParam,
7770
strict bool,
7871
definition string,
7972
extensionName string,
@@ -198,7 +191,7 @@ func (c *CreateFunction) WithResolvedChildren(children []any) (any, error) {
198191
// the number of default values can be fewer but cannot be more.
199192
return nil, ErrVitessChildCount.New(len(c.Parameters), len(children))
200193
}
201-
newParams := make([]RoutineArg, len(c.Parameters))
194+
newParams := make([]RoutineParam, len(c.Parameters))
202195
childIdx := 0
203196
for i, param := range c.Parameters {
204197
newParams[i] = param

‎server/node/create_procedure.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type CreateProcedure struct {
3333
ProcedureName string
3434
SchemaName string
3535
Replace bool
36-
Parameters []RoutineArg
36+
Parameters []RoutineParam
3737
Statements []plpgsql.InterpreterOperation
3838
ExtensionName string
3939
ExtensionSymbol string
@@ -50,7 +50,7 @@ func NewCreateProcedure(
5050
procedureName string,
5151
schemaName string,
5252
replace bool,
53-
params []RoutineArg,
53+
params []RoutineParam,
5454
definition string,
5555
extensionName string,
5656
extensionSymbol string,
@@ -167,7 +167,7 @@ func (c *CreateProcedure) WithResolvedChildren(children []any) (any, error) {
167167
// the number of default values can be fewer but cannot be more.
168168
return nil, ErrVitessChildCount.New(len(c.Parameters), len(children))
169169
}
170-
newParams := make([]RoutineArg, len(c.Parameters))
170+
newParams := make([]RoutineParam, len(c.Parameters))
171171
childIdx := 0
172172
for i, param := range c.Parameters {
173173
newParams[i] = param

‎server/node/drop_function.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,16 @@ import (
2525
"github.com/dolthub/doltgresql/core/id"
2626
)
2727

28+
// RoutineWithParams represent a function or a procedure with schema name, routine name and its parameters.
29+
type RoutineWithParams struct {
30+
SchemaName string
31+
RoutineName string
32+
Args []RoutineParam
33+
}
34+
2835
// DropFunction implements DROP FUNCTION.
2936
type DropFunction struct {
30-
RoutinesWithArgs []*RoutineWithArgs
37+
RoutinesWithArgs []*RoutineWithParams
3138
IfExists bool
3239
Cascade bool
3340
}
@@ -36,7 +43,7 @@ var _ sql.ExecSourceRel = (*DropFunction)(nil)
3643
var _ vitess.Injectable = (*DropFunction)(nil)
3744

3845
// NewDropFunction returns a new *DropFunction.
39-
func NewDropFunction(ifExists bool, routinesWithArgs []*RoutineWithArgs, cascade bool) *DropFunction {
46+
func NewDropFunction(ifExists bool, routinesWithArgs []*RoutineWithParams, cascade bool) *DropFunction {
4047
return &DropFunction{
4148
IfExists: ifExists,
4249
RoutinesWithArgs: routinesWithArgs,
@@ -99,7 +106,7 @@ func (d *DropFunction) WithResolvedChildren(children []any) (any, error) {
99106
return d, nil
100107
}
101108

102-
func dropFunction(ctx *sql.Context, funcColl *functions.Collection, fn *RoutineWithArgs, ifExists bool) error {
109+
func dropFunction(ctx *sql.Context, funcColl *functions.Collection, fn *RoutineWithParams, ifExists bool) error {
103110
// TODO: provide db
104111
schema, err := core.GetSchemaName(ctx, nil, fn.SchemaName)
105112
if err != nil {

‎server/node/drop_procedure.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727

2828
// DropProcedure implements DROP PROCEDURE.
2929
type DropProcedure struct {
30-
RoutinesWithArgs []*RoutineWithArgs
30+
RoutinesWithArgs []*RoutineWithParams
3131
IfExists bool
3232
Cascade bool
3333
}
@@ -36,7 +36,7 @@ var _ sql.ExecSourceRel = (*DropProcedure)(nil)
3636
var _ vitess.Injectable = (*DropProcedure)(nil)
3737

3838
// NewDropProcedure returns a new *DropProcedure.
39-
func NewDropProcedure(ifExists bool, routinesWithArgs []*RoutineWithArgs, cascade bool) *DropProcedure {
39+
func NewDropProcedure(ifExists bool, routinesWithArgs []*RoutineWithParams, cascade bool) *DropProcedure {
4040
return &DropProcedure{
4141
IfExists: ifExists,
4242
RoutinesWithArgs: routinesWithArgs,
@@ -99,7 +99,7 @@ func (d *DropProcedure) WithResolvedChildren(children []any) (any, error) {
9999
return d, nil
100100
}
101101

102-
func dropProcedure(ctx *sql.Context, procColl *procedures.Collection, fn *RoutineWithArgs, ifExists bool) error {
102+
func dropProcedure(ctx *sql.Context, procColl *procedures.Collection, fn *RoutineWithParams, ifExists bool) error {
103103
// TODO: provide db
104104
schema, err := core.GetSchemaName(ctx, nil, fn.SchemaName)
105105
if err != nil {

‎server/node/return.go‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package node
1717
import (
1818
"fmt"
1919

20+
"github.com/cockroachdb/errors"
2021
"github.com/dolthub/go-mysql-server/sql"
2122
vitess "github.com/dolthub/vitess/go/vt/sqlparser"
2223
)
@@ -28,6 +29,7 @@ type Return struct {
2829
}
2930

3031
var _ sql.ExecSourceRel = (*Return)(nil)
32+
var _ sql.Expressioner = (*Return)(nil)
3133
var _ vitess.Injectable = (*Return)(nil)
3234

3335
// NewReturn creates a new *Return node.
@@ -58,12 +60,7 @@ func (r *Return) Resolved() bool {
5860

5961
// RowIter implements the interface sql.ExecSourceRel.
6062
func (r *Return) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) {
61-
// TODO: this cannot be called as we replace RETURN with SELECT to be able to parse the expression
62-
//val, err := r.Expr.Eval(ctx, row)
63-
//if err != nil {
64-
// return nil, err
65-
//}
66-
return sql.RowsToRowIter(), nil
63+
return nil, errors.Errorf(`cannot call RowIter on Return node`)
6764
}
6865

6966
// String implements the interface sql.ExecSourceRel.
@@ -99,3 +96,19 @@ func (r *Return) WithResolvedChildren(children []any) (any, error) {
9996
nr.Expr = children[0].(sql.Expression)
10097
return &nr, nil
10198
}
99+
100+
// Expressions implements the interface sql.Expressioner.
101+
func (r *Return) Expressions() []sql.Expression {
102+
return []sql.Expression{r.Expr}
103+
}
104+
105+
// WithExpressions implements the interface sql.Expressioner.
106+
func (r *Return) WithExpressions(exprs ...sql.Expression) (sql.Node, error) {
107+
if len(exprs) != 1 {
108+
return nil, sql.ErrInvalidChildrenNumber.New(r, len(exprs), 1)
109+
}
110+
111+
nr := *r
112+
nr.Expr = exprs[0]
113+
return &nr, nil
114+
}

0 commit comments

Comments
 (0)