Skip to content

Commit

Permalink
sql-server: Fix regression: Respect autocommit config.
Browse files Browse the repository at this point in the history
In the migration to *SqlEngine in 0.34.4 to 0.34.5, the server config's respect
for the default value for autocommit regressed. In 0.34.5 - 0.35.3,
`autocommit` is always turned on by default for incoming connections,
regardless of the setting in the config.yaml file or the flag
`--no-auto-commit` being passed at the command line.
  • Loading branch information
reltuk committed Jan 10, 2022
1 parent f152abe commit 7b2abff
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 7 deletions.
6 changes: 3 additions & 3 deletions go/cmd/dolt/commands/engine/sqlengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func NewSqlEngine(
return &SqlEngine{
dbs: nameToDB,
contextFactory: newSqlContext(sess, initialDb),
dsessFactory: newDoltSession(pro, mrEnv.Config()),
dsessFactory: newDoltSession(pro, mrEnv.Config(), autocommit),
engine: engine,
resultFormat: format,
}, nil
Expand Down Expand Up @@ -256,7 +256,7 @@ func newSqlContext(sess *dsess.DoltSession, initialDb string) func(ctx context.C
}
}

func newDoltSession(pro dsqle.DoltDatabaseProvider, config config.ReadWriteConfig) func(ctx context.Context, mysqlSess *sql.BaseSession, dbs []sql.Database) (*dsess.DoltSession, error) {
func newDoltSession(pro dsqle.DoltDatabaseProvider, config config.ReadWriteConfig, autocommit bool) func(ctx context.Context, mysqlSess *sql.BaseSession, dbs []sql.Database) (*dsess.DoltSession, error) {
return func(ctx context.Context, mysqlSess *sql.BaseSession, dbs []sql.Database) (*dsess.DoltSession, error) {
ddbs := dsqle.DbsAsDSQLDBs(dbs)
states, err := getDbStates(ctx, ddbs)
Expand All @@ -270,7 +270,7 @@ func newDoltSession(pro dsqle.DoltDatabaseProvider, config config.ReadWriteConfi
}

// TODO: this should just be the session default like it is with MySQL
err = dsess.SetSessionVariable(sql.NewContext(ctx), sql.AutoCommitSessionVar, true)
err = dsess.SetSessionVariable(sql.NewContext(ctx), sql.AutoCommitSessionVar, autocommit)
if err != nil {
return nil, err
}
Expand Down
7 changes: 6 additions & 1 deletion integration-tests/bats/sql-server-mysql.expect
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ expect {
failed { exit 1; }
}
expect {
"*4*" { send "exit\r"; }
"*4*" { send "select @@autocommit;\r"; }
timeout { exit 1; }
failed { exit 1; }
}
expect {
"*0 |*" { send "exit\r"; }
timeout { exit 1; }
failed { exit 1; }
}
Expand Down
5 changes: 2 additions & 3 deletions integration-tests/bats/sql-server.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1418,7 +1418,7 @@ databases:
}

@test "sql-server: run mysql from shell" {
skip "test mysql client from shell fails after v0.34.9"
skiponwindows "Has dependencies that are not installed on Windows CI"

cd repo1
dolt sql -q "create table r1t_one (id1 int primary key, col1 varchar(20));"
Expand All @@ -1434,8 +1434,7 @@ databases:
dolt commit -am "create three tables"

cd ..
start_sql_server
# server_query "repo1" 1 "show databases" "Database\ninformation_schema\nrepo1\nrepo2"
start_sql_server_with_args --user dolt -ltrace --no-auto-commit

run expect $BATS_TEST_DIRNAME/sql-server-mysql.expect $PORT repo1
[ "$status" -eq 0 ]
Expand Down

0 comments on commit 7b2abff

Please sign in to comment.