Merge pull request #1753 from flipt-io/configure-prepared-statements

feat: configure use of prepared statements

cmd/flipt/server.go

@@ -25,15 +25,19 @@ func fliptServer(logger *zap.Logger, cfg *config.Config) (*server.Server, func()
 		return nil, nil, fmt.Errorf("opening db: %w", err)
 	}
 
+	logger.Debug("constructing builder", zap.Bool("prepared_statements", cfg.Database.PreparedStatementsEnabled))
+
+	builder := sql.BuilderFor(db, driver, cfg.Database.PreparedStatementsEnabled)
+
 	var store storage.Store
 
 	switch driver {
 	case sql.SQLite:
-		store = sqlite.NewStore(db, logger)
+		store = sqlite.NewStore(db, builder, logger)
 	case sql.Postgres, sql.CockroachDB:
-		store = postgres.NewStore(db, logger)
+		store = postgres.NewStore(db, builder, logger)
 	case sql.MySQL:
-		store = mysql.NewStore(db, logger)
+		store = mysql.NewStore(db, builder, logger)
 	}
 
 	return server.New(logger, store), func() { _ = db.Close() }, nil

config/flipt.schema.cue

@@ -101,6 +101,7 @@ import "strings"
 		max_idle_conn?:     int | *2
 		max_open_conn?:     int
 		conn_max_lifetime?: int
+		prepared_statements_enabled?: boolean | *true
 	}
 
 	_#lower: ["debug", "error", "fatal", "info", "panic", "trace", "warn"]

config/flipt.schema.json

@@ -325,6 +325,9 @@
         },
         "conn_max_lifetime": {
           "type": "integer"
+        },
+        "prepared_statements_enabled": {
+          "type": "boolean"
         }
       },
       "required": [],

internal/cmd/auth.go

@@ -20,7 +20,6 @@ import (
 	"go.flipt.io/flipt/internal/storage/auth/memory"
 	authsql "go.flipt.io/flipt/internal/storage/auth/sql"
 	oplocksql "go.flipt.io/flipt/internal/storage/oplock/sql"
-	fliptsql "go.flipt.io/flipt/internal/storage/sql"
 	rpcauth "go.flipt.io/flipt/rpc/flipt/auth"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
@@ -43,18 +42,17 @@ func authenticationGRPC(
 		}, nil, func(ctx context.Context) error { return nil }, nil
 	}
 
-	db, driver, shutdown, err := getDB(ctx, logger, cfg, forceMigrate)
+	_, builder, driver, shutdown, err := getDB(ctx, logger, cfg, forceMigrate)
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
 	var (
-		authCfg    = cfg.Authentication
-		sqlBuilder = fliptsql.BuilderFor(db, driver)
-		store      = authsql.NewStore(driver, sqlBuilder, logger)
-		oplock     = oplocksql.New(logger, driver, sqlBuilder)
-		public     = public.NewServer(logger, authCfg)
-		register   = grpcRegisterers{
+		authCfg  = cfg.Authentication
+		store    = authsql.NewStore(driver, builder, logger)
+		oplock   = oplocksql.New(logger, driver, builder)
+		public   = public.NewServer(logger, authCfg)
+		register = grpcRegisterers{
 			public,
 			auth.NewServer(logger, store, auth.WithAuditLoggingEnabled(cfg.Audit.Enabled())),
 		}

internal/cmd/grpc.go

@@ -10,6 +10,7 @@ import (
 	"sync"
 	"time"
 
+	sq "github.com/Masterminds/squirrel"
 	"go.flipt.io/flipt/internal/config"
 	"go.flipt.io/flipt/internal/containers"
 	"go.flipt.io/flipt/internal/info"
@@ -116,7 +117,7 @@ func NewGRPCServer(
 
 	switch cfg.Storage.Type {
 	case "", config.DatabaseStorageType:
-		db, driver, shutdown, err := getDB(ctx, logger, cfg, forceMigrate)
+		db, builder, driver, shutdown, err := getDB(ctx, logger, cfg, forceMigrate)
 		if err != nil {
 			return nil, err
 		}
@@ -125,11 +126,11 @@ func NewGRPCServer(
 
 		switch driver {
 		case fliptsql.SQLite:
-			store = sqlite.NewStore(db, logger)
+			store = sqlite.NewStore(db, builder, logger)
 		case fliptsql.Postgres, fliptsql.CockroachDB:
-			store = postgres.NewStore(db, logger)
+			store = postgres.NewStore(db, builder, logger)
 		case fliptsql.MySQL:
-			store = mysql.NewStore(db, logger)
+			store = mysql.NewStore(db, builder, logger)
 		default:
 			return nil, fmt.Errorf("unsupported driver: %s", driver)
 		}
@@ -390,14 +391,15 @@ func (s *GRPCServer) onShutdown(fn errFunc) {
 }
 
 var (
-	once   sync.Once
-	db     *sql.DB
-	driver fliptsql.Driver
-	dbFunc errFunc = func(context.Context) error { return nil }
-	dbErr  error
+	once    sync.Once
+	db      *sql.DB
+	builder sq.StatementBuilderType
+	driver  fliptsql.Driver
+	dbFunc  errFunc = func(context.Context) error { return nil }
+	dbErr   error
 )
 
-func getDB(ctx context.Context, logger *zap.Logger, cfg *config.Config, forceMigrate bool) (*sql.DB, fliptsql.Driver, errFunc, error) {
+func getDB(ctx context.Context, logger *zap.Logger, cfg *config.Config, forceMigrate bool) (*sql.DB, sq.StatementBuilderType, fliptsql.Driver, errFunc, error) {
 	once.Do(func() {
 		migrator, err := fliptsql.NewMigrator(*cfg, logger)
 		if err != nil {
@@ -419,6 +421,10 @@ func getDB(ctx context.Context, logger *zap.Logger, cfg *config.Config, forceMig
 			return
 		}
 
+		logger.Debug("constructing builder", zap.Bool("prepared_statements", cfg.Database.PreparedStatementsEnabled))
+
+		builder = fliptsql.BuilderFor(db, driver, cfg.Database.PreparedStatementsEnabled)
+
 		dbFunc = func(context.Context) error {
 			return db.Close()
 		}
@@ -432,5 +438,5 @@ func getDB(ctx context.Context, logger *zap.Logger, cfg *config.Config, forceMig
 		}
 	})
 
-	return db, driver, dbFunc, dbErr
+	return db, builder, driver, dbFunc, dbErr
 }

internal/config/config_test.go

@@ -261,8 +261,9 @@ func defaultConfig() *Config {
 		},
 
 		Database: DatabaseConfig{
-			URL:         "file:/var/opt/flipt/flipt.db",
-			MaxIdleConn: 2,
+			URL:                       "file:/var/opt/flipt/flipt.db",
+			MaxIdleConn:               2,
+			PreparedStatementsEnabled: true,
 		},
 
 		Meta: MetaConfig{
@@ -419,13 +420,14 @@ func TestLoad(t *testing.T) {
 			expected: func() *Config {
 				cfg := defaultConfig()
 				cfg.Database = DatabaseConfig{
-					Protocol:    DatabaseMySQL,
-					Host:        "localhost",
-					Port:        3306,
-					User:        "flipt",
-					Password:    "s3cr3t!",
-					Name:        "flipt",
-					MaxIdleConn: 2,
+					Protocol:                  DatabaseMySQL,
+					Host:                      "localhost",
+					Port:                      3306,
+					User:                      "flipt",
+					Password:                  "s3cr3t!",
+					Name:                      "flipt",
+					MaxIdleConn:               2,
+					PreparedStatementsEnabled: true,
 				}
 				return cfg
 			},
@@ -591,10 +593,11 @@ func TestLoad(t *testing.T) {
 					Type: StorageType("database"),
 				}
 				cfg.Database = DatabaseConfig{
-					URL:             "postgres://postgres@localhost:5432/flipt?sslmode=disable",
-					MaxIdleConn:     10,
-					MaxOpenConn:     50,
-					ConnMaxLifetime: 30 * time.Minute,
+					URL:                       "postgres://postgres@localhost:5432/flipt?sslmode=disable",
+					MaxIdleConn:               10,
+					MaxOpenConn:               50,
+					ConnMaxLifetime:           30 * time.Minute,
+					PreparedStatementsEnabled: true,
 				}
 				cfg.Meta = MetaConfig{
 					CheckForUpdates:  false,

internal/config/database.go

@@ -27,16 +27,17 @@ const (
 //
 // Flipt currently supports SQLite, Postgres and MySQL backends.
 type DatabaseConfig struct {
-	URL             string           `json:"url,omitempty" mapstructure:"url"`
-	MaxIdleConn     int              `json:"maxIdleConn,omitempty" mapstructure:"max_idle_conn"`
-	MaxOpenConn     int              `json:"maxOpenConn,omitempty" mapstructure:"max_open_conn"`
-	ConnMaxLifetime time.Duration    `json:"connMaxLifetime,omitempty" mapstructure:"conn_max_lifetime"`
-	Name            string           `json:"name,omitempty" mapstructure:"name"`
-	User            string           `json:"user,omitempty" mapstructure:"user"`
-	Password        string           `json:"password,omitempty" mapstructure:"password"`
-	Host            string           `json:"host,omitempty" mapstructure:"host"`
-	Port            int              `json:"port,omitempty" mapstructure:"port"`
-	Protocol        DatabaseProtocol `json:"protocol,omitempty" mapstructure:"protocol"`
+	URL                       string           `json:"url,omitempty" mapstructure:"url"`
+	MaxIdleConn               int              `json:"maxIdleConn,omitempty" mapstructure:"max_idle_conn"`
+	MaxOpenConn               int              `json:"maxOpenConn,omitempty" mapstructure:"max_open_conn"`
+	ConnMaxLifetime           time.Duration    `json:"connMaxLifetime,omitempty" mapstructure:"conn_max_lifetime"`
+	Name                      string           `json:"name,omitempty" mapstructure:"name"`
+	User                      string           `json:"user,omitempty" mapstructure:"user"`
+	Password                  string           `json:"password,omitempty" mapstructure:"password"`
+	Host                      string           `json:"host,omitempty" mapstructure:"host"`
+	Port                      int              `json:"port,omitempty" mapstructure:"port"`
+	Protocol                  DatabaseProtocol `json:"protocol,omitempty" mapstructure:"protocol"`
+	PreparedStatementsEnabled bool             `json:"preparedStatementsEnabled,omitempty" mapstructure:"prepared_statements_enabled"`
 }
 
 func (c *DatabaseConfig) setDefaults(v *viper.Viper) {
@@ -54,6 +55,8 @@ func (c *DatabaseConfig) setDefaults(v *viper.Viper) {
 	if setDefaultURL {
 		v.SetDefault("db.url", "file:/var/opt/flipt/flipt.db")
 	}
+
+	v.SetDefault("db.prepared_statements_enabled", true)
 }
 
 func (c *DatabaseConfig) deprecations(v *viper.Viper) []deprecation {

internal/storage/auth/sql/store_test.go

@@ -359,7 +359,7 @@ func newTestStore(t *testing.T, seed ...authentication) func(...Option) *Store {
 		storeFn = func(opts ...Option) *Store {
 			return NewStore(
 				db.Driver,
-				storagesql.BuilderFor(db.DB, db.Driver),
+				storagesql.BuilderFor(db.DB, db.Driver, true),
 				logger,
 				opts...,
 			)

internal/storage/oplock/sql/sql_test.go

@@ -21,6 +21,6 @@ func Test_Harness(t *testing.T) {
 		New(
 			logger,
 			db.Driver,
-			storagesql.BuilderFor(db.DB, db.Driver),
+			storagesql.BuilderFor(db.DB, db.Driver, true),
 		))
 }

internal/storage/sql/db.go

@@ -39,8 +39,13 @@ func Open(cfg config.Config, opts ...Option) (*sql.DB, Driver, error) {
 
 // BuilderFor returns a squirrel statement builder which decorates
 // the provided sql.DB configured for the provided driver.
-func BuilderFor(db *sql.DB, driver Driver) sq.StatementBuilderType {
-	builder := sq.StatementBuilder.RunWith(sq.NewStmtCacher(db))
+func BuilderFor(db *sql.DB, driver Driver, preparedStatementsEnabled bool) sq.StatementBuilderType {
+	var brdb sq.BaseRunner = db
+	if preparedStatementsEnabled {
+		brdb = sq.NewStmtCacher(db)
+	}
+
+	builder := sq.StatementBuilder.RunWith(brdb)
 	if driver == Postgres || driver == CockroachDB {
 		builder = builder.PlaceholderFormat(sq.Dollar)
 	}
@@ -203,36 +208,32 @@ func parse(cfg config.Config, opts Options) (Driver, *dburl.URL, error) {
 		return 0, nil, fmt.Errorf("unknown database driver for: %q", url.Driver)
 	}
 
+	v := url.Query()
 	switch driver {
 	case Postgres, CockroachDB:
 		if opts.sslDisabled {
-			v := url.Query()
 			v.Set("sslmode", "disable")
-			url.RawQuery = v.Encode()
-			// we need to re-parse since we modified the query params
-			url, err = dburl.Parse(url.URL.String())
+		}
+
+		// see: https://github.com/lib/pq/issues/389
+		if !cfg.Database.PreparedStatementsEnabled {
+			v.Set("binary_parameters", "yes")
 		}
 	case MySQL:
-		v := url.Query()
 		v.Set("multiStatements", "true")
 		v.Set("parseTime", "true")
 		if !opts.migrate {
 			v.Set("sql_mode", "ANSI")
 		}
-		url.RawQuery = v.Encode()
-		// we need to re-parse since we modified the query params
-		url, err = dburl.Parse(url.URL.String())
-
 	case SQLite:
-		v := url.Query()
 		v.Set("cache", "shared")
 		v.Set("mode", "rwc")
 		v.Set("_fk", "true")
-		url.RawQuery = v.Encode()
-
-		// we need to re-parse since we modified the query params
-		url, err = dburl.Parse(url.URL.String())
 	}
 
+	url.RawQuery = v.Encode()
+	// we need to re-parse since we modified the query params
+	url, err = dburl.Parse(url.URL.String())
+
 	return driver, url, err
 }