refactor after PR review

Author: Gleb Brozhe

.golangci.yml

@@ -295,6 +295,7 @@ issues:
         - predeclared
     - path: _test\.go
       linters:
+        - funlen
         - unused
         - unparam
         - gocritic

internal/balancer/balancer.go

@@ -234,19 +234,10 @@ func New(
 	pool *conn.Pool,
 	opts ...discoveryConfig.Option,
 ) (b *Balancer, finalErr error) {
-	var (
-		onDone = trace.DriverOnBalancerInit(
-			driverConfig.Trace(), &ctx,
-			stack.FunctionID(""),
-			driverConfig.Balancer().String(),
-		)
-		discoveryConfig = discoveryConfig.New(append(opts,
-			discoveryConfig.With(driverConfig.Common),
-			discoveryConfig.WithEndpoint(driverConfig.Endpoint()),
-			discoveryConfig.WithDatabase(driverConfig.Database()),
-			discoveryConfig.WithSecure(driverConfig.Secure()),
-			discoveryConfig.WithMeta(driverConfig.Meta()),
-		)...)
+	onDone := trace.DriverOnBalancerInit(
+		driverConfig.Trace(), &ctx,
+		stack.FunctionID(""),
+		driverConfig.Balancer().String(),
 	)
 	defer func() {
 		onDone(finalErr)
@@ -257,28 +248,43 @@ func New(
 		pool:            pool,
 		localDCDetector: detectLocalDC,
 	}
-	d := internalDiscovery.New(ctx, pool.Get(
-		endpoint.New(driverConfig.Endpoint()),
-	), discoveryConfig)
-
-	b.discoveryClient = d
 
 	if config := driverConfig.Balancer(); config == nil {
 		b.config = balancerConfig.Config{}
 	} else {
 		b.config = *config
 	}
 
+	err := startDiscovery(ctx, b, opts)
+	if err != nil {
+		return nil, err
+	}
+
+	return b, nil
+}
+
+func startDiscovery(ctx context.Context, b *Balancer, opts []discoveryConfig.Option) error {
+	discoveryConfig := discoveryConfig.New(append(opts,
+		discoveryConfig.With(b.driverConfig.Common),
+		discoveryConfig.WithEndpoint(b.driverConfig.Endpoint()),
+		discoveryConfig.WithDatabase(b.driverConfig.Database()),
+		discoveryConfig.WithSecure(b.driverConfig.Secure()),
+		discoveryConfig.WithMeta(b.driverConfig.Meta()),
+	)...)
+	d := internalDiscovery.New(ctx, b.pool.Get(
+		endpoint.New(b.driverConfig.Endpoint()),
+	), discoveryConfig)
+
+	b.discoveryClient = d
+
 	if b.config.SingleConn {
 		b.applyDiscoveredEndpoints(ctx, []endpoint.Endpoint{
-			endpoint.New(driverConfig.Endpoint()),
+			endpoint.New(b.driverConfig.Endpoint()),
 		}, "")
 	} else {
-		// initialization of balancer state
 		if err := b.clusterDiscovery(ctx); err != nil {
-			return nil, xerrors.WithStackTrace(err)
+			return err
 		}
-		// run background discovering
 		if d := discoveryConfig.Interval(); d > 0 {
 			b.discoveryRepeater = repeater.New(xcontext.ValueOnly(ctx),
 				d, b.clusterDiscoveryAttempt,
@@ -288,7 +294,7 @@ func New(
 		}
 	}
 
-	return b, nil
+	return nil
 }
 
 func (b *Balancer) Invoke(

internal/bind/numeric_args.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 	"unicode/utf8"
 
+	"github.com/ydb-platform/ydb-go-sdk/v3/internal/params"
 	"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
 	"github.com/ydb-platform/ydb-go-sdk/v3/internal/xstring"
 	"github.com/ydb-platform/ydb-go-sdk/v3/table"
@@ -16,9 +17,7 @@ func (m NumericArgs) blockID() blockID {
 	return blockYQL
 }
 
-func (m NumericArgs) RewriteQuery(sql string, args ...interface{}) (
-	yql string, newArgs []interface{}, err error,
-) {
+func (m NumericArgs) RewriteQuery(sql string, args ...interface{}) (yql string, newArgs []interface{}, err error) {
 	l := &sqlLexer{
 		src:        sql,
 		stateFn:    numericArgsStateFn,
@@ -29,14 +28,18 @@ func (m NumericArgs) RewriteQuery(sql string, args ...interface{}) (
 		l.stateFn = l.stateFn(l)
 	}
 
-	var (
-		buffer = xstring.Buffer()
-		param  table.ParameterOption
-	)
+	buffer := xstring.Buffer()
 	defer buffer.Free()
 
 	if len(args) > 0 {
-		newArgs = make([]interface{}, len(args))
+		parameters, err := parsePositionalParameters(args)
+		if err != nil {
+			return "", nil, err
+		}
+		newArgs = make([]interface{}, len(parameters))
+		for i, param := range parameters {
+			newArgs[i] = param
+		}
 	}
 
 	for _, p := range l.parts {
@@ -49,38 +52,23 @@ func (m NumericArgs) RewriteQuery(sql string, args ...interface{}) (
 			}
 			if int(p) > len(args) {
 				return "", nil, xerrors.WithStackTrace(
-					fmt.Errorf("%w: $p%d, len(args) = %d", ErrInconsistentArgs, p, len(args)),
+					fmt.Errorf("%w: $%d, len(args) = %d", ErrInconsistentArgs, p, len(args)),
 				)
 			}
-			paramName := "$p" + strconv.Itoa(int(p-1)) //nolint:goconst
-			if newArgs[p-1] == nil {
-				param, err = toYdbParam(paramName, args[p-1])
-				if err != nil {
-					return "", nil, xerrors.WithStackTrace(err)
-				}
-				newArgs[p-1] = param
-				buffer.WriteString(param.Name())
-			} else {
-				buffer.WriteString(newArgs[p-1].(table.ParameterOption).Name())
+			paramIndex := int(p - 1)
+			if newArgs[paramIndex] == nil {
+				return "", nil, xerrors.WithStackTrace(ErrInconsistentArgs)
 			}
+			buffer.WriteString(newArgs[paramIndex].(table.ParameterOption).Name())
 		}
 	}
 
-	for i, p := range newArgs {
-		if p == nil {
-			return "", nil, xerrors.WithStackTrace(
-				fmt.Errorf("%w: $p%d, len(args) = %d", ErrInconsistentArgs, i+1, len(args)),
-			)
-		}
-	}
-
+	yql = buffer.String()
 	if len(newArgs) > 0 {
-		const prefix = "-- origin query with numeric args replacement\n"
-
-		return prefix + buffer.String(), newArgs, nil
+		yql = "-- origin query with numeric args replacement\n" + yql
 	}
 
-	return buffer.String(), newArgs, nil
+	return yql, newArgs, nil
 }
 
 func numericArgsStateFn(l *sqlLexer) stateFn {
@@ -130,6 +118,20 @@ func numericArgsStateFn(l *sqlLexer) stateFn {
 	}
 }
 
+func parsePositionalParameters(args []interface{}) ([]*params.Parameter, error) {
+	newArgs := make([]*params.Parameter, len(args))
+	for i, arg := range args {
+		paramName := fmt.Sprintf("$p%d", i)
+		param, err := toYdbParam(paramName, arg)
+		if err != nil {
+			return nil, err
+		}
+		newArgs[i] = param
+	}
+
+	return newArgs, nil
+}
+
 func numericArgState(l *sqlLexer) stateFn {
 	numbers := ""
 	defer func() {

internal/bind/params.go

@@ -21,7 +21,7 @@ var (
 	errMultipleQueryParameters = errors.New("only one query arg *table.QueryParameters allowed")
 )
 
-//nolint:gocyclo
+//nolint:gocyclo,funlen
 func toValue(v interface{}) (_ types.Value, err error) {
 	if valuer, ok := v.(driver.Valuer); ok {
 		v, err = valuer.Value()
@@ -174,62 +174,70 @@ func toYdbParam(name string, value interface{}) (*params.Parameter, error) {
 	return params.Named(name, v), nil
 }
 
-func Params(args ...interface{}) (parameters []*params.Parameter, _ error) {
-	parameters = make([]*params.Parameter, 0, len(args))
+func Params(args ...interface{}) ([]*params.Parameter, error) {
+	parameters := make([]*params.Parameter, 0, len(args))
 	for i, arg := range args {
+		var newParam *params.Parameter
+		var newParams []*params.Parameter
+		var err error
 		switch x := arg.(type) {
 		case driver.NamedValue:
-			if x.Name == "" {
-				switch xx := x.Value.(type) {
-				case *params.Parameters:
-					if len(args) > 1 {
-						return nil, xerrors.WithStackTrace(errMultipleQueryParameters)
-					}
-					parameters = *xx
-				case *params.Parameter:
-					parameters = append(parameters, xx)
-				default:
-					x.Name = fmt.Sprintf("$p%d", i)
-					param, err := toYdbParam(x.Name, x.Value)
-					if err != nil {
-						return nil, xerrors.WithStackTrace(err)
-					}
-					parameters = append(parameters, param)
-				}
-			} else {
-				param, err := toYdbParam(x.Name, x.Value)
-				if err != nil {
-					return nil, xerrors.WithStackTrace(err)
-				}
-				parameters = append(parameters, param)
-			}
+			newParams, err = paramHandleNamedValue(x, i, len(args))
 		case sql.NamedArg:
 			if x.Name == "" {
 				return nil, xerrors.WithStackTrace(errUnnamedParam)
 			}
-			param, err := toYdbParam(x.Name, x.Value)
-			if err != nil {
-				return nil, xerrors.WithStackTrace(err)
-			}
-			parameters = append(parameters, param)
+			newParam, err = toYdbParam(x.Name, x.Value)
+			newParams = append(newParams, newParam)
 		case *params.Parameters:
 			if len(args) > 1 {
 				return nil, xerrors.WithStackTrace(errMultipleQueryParameters)
 			}
 			parameters = *x
 		case *params.Parameter:
-			parameters = append(parameters, x)
+			newParams = append(newParams, x)
 		default:
-			param, err := toYdbParam(fmt.Sprintf("$p%d", i), x)
-			if err != nil {
-				return nil, xerrors.WithStackTrace(err)
-			}
-			parameters = append(parameters, param)
+			newParam, err = toYdbParam(fmt.Sprintf("$p%d", i), x)
+			newParams = append(newParams, newParam)
+		}
+		if err != nil {
+			return nil, xerrors.WithStackTrace(err)
 		}
+		parameters = append(parameters, newParams...)
 	}
 	sort.Slice(parameters, func(i, j int) bool {
 		return parameters[i].Name() < parameters[j].Name()
 	})
 
 	return parameters, nil
 }
+
+func paramHandleNamedValue(arg driver.NamedValue, paramNumber, argsLen int) ([]*params.Parameter, error) {
+	if arg.Name == "" {
+		switch x := arg.Value.(type) {
+		case *params.Parameters:
+			if argsLen > 1 {
+				return nil, xerrors.WithStackTrace(errMultipleQueryParameters)
+			}
+
+			return *x, nil
+		case *params.Parameter:
+			return []*params.Parameter{x}, nil
+		default:
+			arg.Name = fmt.Sprintf("$p%d", paramNumber)
+			param, err := toYdbParam(arg.Name, arg.Value)
+			if err != nil {
+				return nil, xerrors.WithStackTrace(err)
+			}
+
+			return []*params.Parameter{param}, nil
+		}
+	} else {
+		param, err := toYdbParam(arg.Name, arg.Value)
+		if err != nil {
+			return nil, xerrors.WithStackTrace(err)
+		}
+
+		return []*params.Parameter{param}, nil
+	}
+}

internal/conn/conn.go

@@ -362,6 +362,7 @@ func (c *conn) Invoke(
 	return err
 }
 
+//nolint:funlen
 func (c *conn) NewStream(
 	ctx context.Context,
 	desc *grpc.StreamDesc,