From 51edd8decbdac79b3e207bd94562dc8a78fc32f7 Mon Sep 17 00:00:00 2001
From: Jason Fulghum <jason@dolthub.com>
Date: Tue, 14 May 2024 15:21:33 -0700
Subject: [PATCH] Bug fix: dolt sql-server should initialize persisted global
 variables from its local config store

---
 go/cmd/dolt/commands/sqlserver/server.go    | 14 ++++++++++-
 go/cmd/dolt/commands/sqlserver/sqlserver.go |  4 ---
 integration-tests/bats/sql-server.bats      | 28 ++++++++++++++++++++-
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/go/cmd/dolt/commands/sqlserver/server.go b/go/cmd/dolt/commands/sqlserver/server.go
index 0e029ca06f..b62c73f416 100644
--- a/go/cmd/dolt/commands/sqlserver/server.go
+++ b/go/cmd/dolt/commands/sqlserver/server.go
@@ -45,6 +45,7 @@ import (
 	"github.com/dolthub/dolt/go/cmd/dolt/commands/engine"
 	eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1"
 	remotesapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/remotesapi/v1alpha1"
+	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/remotesrv"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle"
@@ -154,14 +155,25 @@ func ConfigureServices(
 
 	fs := dEnv.FS
 	InitDataDir := &svcs.AnonService{
-		InitF: func(context.Context) (err error) {
+		InitF: func(ctx context.Context) (err error) {
 			if len(serverConfig.DataDir()) > 0 && serverConfig.DataDir() != "." {
 				fs, err = dEnv.FS.WithWorkingDir(serverConfig.DataDir())
 				if err != nil {
 					return err
 				}
+				// If datadir has changed, then reload the DoltEnv to ensure its local
+				// configuration store gets configured correctly
 				dEnv.FS = fs
+				dEnv = env.Load(ctx, dEnv.GetUserHomeDir, fs, doltdb.LocalDirDoltDB, dEnv.Version)
+
+				// If the datadir has changed, then we need to load any persisted global variables
+				// from the new datadir's local configuration store
+				err = dsess.InitPersistedSystemVars(dEnv)
+				if err != nil {
+					logrus.Errorf("failed to load persisted global variables: %s\n", err.Error())
+				}
 			}
+			dEnv.Config.SetFailsafes(env.DefaultFailsafeConfig)
 			return nil
 		},
 	}
diff --git a/go/cmd/dolt/commands/sqlserver/sqlserver.go b/go/cmd/dolt/commands/sqlserver/sqlserver.go
index ed42c8b24c..8a87035996 100644
--- a/go/cmd/dolt/commands/sqlserver/sqlserver.go
+++ b/go/cmd/dolt/commands/sqlserver/sqlserver.go
@@ -207,9 +207,6 @@ func (cmd SqlServerCmd) Exec(ctx context.Context, commandStr string, args []stri
 		}
 	}()
 
-	// We need a username and password for many SQL commands, so set defaults if they don't exist
-	dEnv.Config.SetFailsafes(env.DefaultFailsafeConfig)
-
 	err := StartServer(newCtx, cmd.VersionStr, commandStr, args, dEnv, controller)
 	if err != nil {
 		cli.Println(color.RedString(err.Error()))
@@ -235,7 +232,6 @@ func validateSqlServerArgs(apr *argparser.ArgParseResults) error {
 func StartServer(ctx context.Context, versionStr, commandStr string, args []string, dEnv *env.DoltEnv, controller *svcs.Controller) error {
 	ap := SqlServerCmd{}.ArgParser()
 	help, _ := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, sqlServerDocs, ap))
-
 	serverConfig, err := ServerConfigFromArgs(ap, help, args, dEnv)
 	if err != nil {
 		return err
diff --git a/integration-tests/bats/sql-server.bats b/integration-tests/bats/sql-server.bats
index dab87d4871..0bd39f5fa4 100644
--- a/integration-tests/bats/sql-server.bats
+++ b/integration-tests/bats/sql-server.bats
@@ -598,7 +598,6 @@ SQL
 
      run dolt sql -q "SELECT * FROM test"
      [ $status -eq 0 ]
-     echo $output
      [[ $output =~ " 1 " ]] || false
      [[ $output =~ " 2 " ]] || false
      [[ $output =~ " 3 " ]] || false
@@ -1883,3 +1882,30 @@ behavior:
     [ $status -eq 0 ]
     [[ "$output" =~ "__dolt_local_user__@localhost" ]] || false
 }
+
+@test "sql-server: --data-dir used to load persisted system variables" {
+    prevWd=$(pwd)
+    baseDir=$(mktemp -d)
+
+    # Initialize a Dolt directory and persist a global variable
+    cd $baseDir
+    dolt init
+    dolt sql -q "SET @@PERSIST.log_bin=1;"
+    run cat .dolt/config.json
+    [ $status -eq 0 ]
+    [[ "$output" =~ "\"sqlserver.global.log_bin\":\"1\"" ]] || false
+
+    # Start a sql-server and make sure the persisted global was loaded
+    cd $prevWd
+    PORT=$( definePORT )
+    dolt sql-server --data-dir=$baseDir --host 0.0.0.0 --port=$PORT &
+    SERVER_PID=$!
+    SQL_USER='root'
+    wait_for_connection $PORT 7500
+
+    run dolt --data-dir=$baseDir sql -q "select @@log_bin"
+    [ $status -eq 0 ]
+    [[ "$output" =~ "1" ]] || false
+}
+
+