From 790219c70bc88903f99e696f942b36ba5e83a5e3 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 22 May 2023 16:23:45 -0700 Subject: [PATCH 01/64] Convert Add command to use CliContext. --- go/cmd/dolt/commands/add.go | 65 ++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index ebbb266031..1de9649f2b 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -15,13 +15,14 @@ package commands import ( + "bytes" "context" + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" - "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/utils/argparser" ) @@ -59,40 +60,50 @@ func (cmd AddCmd) ArgParser() *argparser.ArgParser { return cli.CreateAddArgParser() } +// Generate the query that will call the `DOLT_ADD` stored proceudre. +// This is safe because the only inputs are flag names and validated table names. +func generateSql(apr *argparser.ArgParseResults) string { + var buffer bytes.Buffer + buffer.WriteString("CALL DOLT_ADD('") + if apr.Contains(cli.AllFlag) { + buffer.WriteString("-A', '") + } + if apr.Contains(cli.ForceFlag) { + buffer.WriteString("-f', '") + } + for i := 0; i < apr.NArg()-1; i++ { + buffer.WriteString(apr.Arg(i)) + buffer.WriteString("', '") + } + buffer.WriteString(apr.Arg(apr.NArg() - 1)) + buffer.WriteString("')") + return buffer.String() +} + // Exec executes the command func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int { + queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) + if err != nil { + + } + if closeFunc != nil { + defer closeFunc() + } + ap := cli.CreateAddArgParser() helpPr, _ := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, addDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, helpPr) - allFlag := apr.Contains(cli.AllFlag) - - if dEnv.IsLocked() { - return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), helpPr) - } - - roots, err := dEnv.Roots(ctx) - if err != nil { - return HandleStageError(err) - } - - if apr.NArg() == 0 && !allFlag { - cli.Println("Nothing specified, nothing added.\n Maybe you wanted to say 'dolt add .'?") - } else if allFlag || apr.NArg() == 1 && apr.Arg(0) == "." { - roots, err = actions.StageAllTables(ctx, roots, !apr.Contains(cli.ForceFlag)) - if err != nil { - return HandleStageError(err) - } - } else { - roots, err = actions.StageTables(ctx, roots, apr.Args, !apr.Contains(cli.ForceFlag)) - if err != nil { - return HandleStageError(err) + for _, tableName := range apr.Args { + if !doltdb.IsValidTableName(tableName) { + return HandleVErrAndExitCode(errhand.BuildDError("'%s' is not a valid table name", tableName).Build(), nil) } } - err = dEnv.UpdateRoots(ctx, roots) + _, _, err = queryist.Query(sqlCtx, generateSql(apr)) if err != nil { - return HandleStageError(err) + cli.PrintErrln(errhand.VerboseErrorFromError(err)) + return 1 } return 0 @@ -144,7 +155,3 @@ func toStageVErr(err error) errhand.VerboseError { return errhand.BuildDError("Unknown error").AddCause(err).Build() } } - -func HandleDocTableVErrAndExitCode() int { - return HandleVErrAndExitCode(errhand.BuildDError("'%s' is not a valid table name", doltdb.DocTableName).Build(), nil) -} From d6c05dc7dc2ccfd395436a1fc70b9844379b1fb9 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 22 May 2023 17:02:51 -0700 Subject: [PATCH 02/64] Allow adding "." (shorthand for adding all tables.) --- go/cmd/dolt/commands/add.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 1de9649f2b..4a1d7a39c1 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -95,7 +95,7 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE apr := cli.ParseArgsOrDie(ap, args, helpPr) for _, tableName := range apr.Args { - if !doltdb.IsValidTableName(tableName) { + if tableName != "." && !doltdb.IsValidTableName(tableName) { return HandleVErrAndExitCode(errhand.BuildDError("'%s' is not a valid table name", tableName).Build(), nil) } } From dea001ec510fce26dabc47fa876ac1292784f78e Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 22 May 2023 17:10:43 -0700 Subject: [PATCH 03/64] Iterate over result rows. --- go/cmd/dolt/commands/add.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 4a1d7a39c1..77b6d71e16 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" + "io" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" @@ -100,12 +101,23 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE } } - _, _, err = queryist.Query(sqlCtx, generateSql(apr)) + _, rowIter, err := queryist.Query(sqlCtx, generateSql(apr)) if err != nil { cli.PrintErrln(errhand.VerboseErrorFromError(err)) return 1 } + for { + _, err := rowIter.Next(sqlCtx) + if err == io.EOF { + return 0 + } + if err != nil { + cli.PrintErrln(errhand.VerboseErrorFromError(err)) + return 1 + } + } + return 0 } From 97ce70e86e928ae528cd38ed23684ded443fa1f8 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 22 May 2023 17:11:02 -0700 Subject: [PATCH 04/64] Remove unreachable return. --- go/cmd/dolt/commands/add.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 77b6d71e16..9accf7b504 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -117,8 +117,6 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE return 1 } } - - return 0 } func HandleStageError(err error) int { From 65a722ba2675997dc2d8bb789ee333e97ce547ad Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 22 May 2023 17:22:10 -0700 Subject: [PATCH 05/64] Update query generation. --- go/cmd/dolt/commands/add.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 9accf7b504..7f5198778d 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -65,19 +65,30 @@ func (cmd AddCmd) ArgParser() *argparser.ArgParser { // This is safe because the only inputs are flag names and validated table names. func generateSql(apr *argparser.ArgParseResults) string { var buffer bytes.Buffer - buffer.WriteString("CALL DOLT_ADD('") + var first bool + first = true + buffer.WriteString("CALL DOLT_ADD(") + + write := func(s string) { + if !first { + buffer.WriteString(", ") + } + buffer.WriteString("'") + buffer.WriteString(s) + buffer.WriteString("'") + first = false + } + if apr.Contains(cli.AllFlag) { - buffer.WriteString("-A', '") + write("-A") } if apr.Contains(cli.ForceFlag) { - buffer.WriteString("-f', '") + write("-f") } - for i := 0; i < apr.NArg()-1; i++ { - buffer.WriteString(apr.Arg(i)) - buffer.WriteString("', '") + for _, arg := range apr.Args { + write(arg) } - buffer.WriteString(apr.Arg(apr.NArg() - 1)) - buffer.WriteString("')") + buffer.WriteString(")") return buffer.String() } From d91527a266eb6b9b35536a17bae4af72fd65be15 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 12:52:52 -0700 Subject: [PATCH 06/64] Ensure that `dolt add` closes the RowIter in order to persist changes. --- go/cmd/dolt/commands/add.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 7f5198778d..dfd2b609cc 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -17,9 +17,11 @@ package commands import ( "bytes" "context" - "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "io" + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" From 91e9b89940790aa8141e8b3aca89a87e400f1e27 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 12:58:44 -0700 Subject: [PATCH 07/64] Add missing return statement. --- go/cmd/dolt/commands/add.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index dfd2b609cc..9374a529d6 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -130,6 +130,8 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE return 1 } } + + return 0 } func HandleStageError(err error) int { From e475c91fe88713a1db0ed445495087e21d05f4f3 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 15:40:40 -0700 Subject: [PATCH 08/64] Have AddCmd implement RepoNotRequiredCommand --- go/cmd/dolt/commands/add.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 9374a529d6..abd56a0e7a 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -19,13 +19,11 @@ import ( "context" "io" - "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/utils/argparser" ) @@ -44,6 +42,12 @@ The dolt status command can be used to obtain a summary of which tables have cha type AddCmd struct{} +var _ cli.RepoNotRequiredCommand = AddCmd{} + +func (cmd AddCmd) RequiresRepo() bool { + return false +} + // Name is returns the name of the Dolt cli command. This is what is used on the command line to invoke the command func (cmd AddCmd) Name() string { return "add" From ff695a58b93f315e93b7a70d8adb7e78b6ba58e1 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 15:42:48 -0700 Subject: [PATCH 09/64] Comment on generateSql should conform to godocs --- go/cmd/dolt/commands/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index abd56a0e7a..72b29ed16a 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -67,8 +67,8 @@ func (cmd AddCmd) ArgParser() *argparser.ArgParser { return cli.CreateAddArgParser() } -// Generate the query that will call the `DOLT_ADD` stored proceudre. -// This is safe because the only inputs are flag names and validated table names. +// generateSql returns the query that will call the `DOLT_ADD` stored proceudre. +// This function assumes that the inputs are validated table names, which cannot contain quotes. func generateSql(apr *argparser.ArgParseResults) string { var buffer bytes.Buffer var first bool From 75622dfc639c047708b9b3154cbad7493ceb7f84 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 15:43:37 -0700 Subject: [PATCH 10/64] Don't ignore error --- go/cmd/dolt/commands/add.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 72b29ed16a..d6d9b85edb 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -102,7 +102,8 @@ func generateSql(apr *argparser.ArgParseResults) string { func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int { queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) if err != nil { - + cli.PrintErrln(errhand.VerboseErrorFromError(err)) + return 1 } if closeFunc != nil { defer closeFunc() From 19da9e75a3dd6c22eb0bdafd0f33f228f0d5b470 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 15:50:33 -0700 Subject: [PATCH 11/64] Ensure that dolt add closes the RowIter in order to persist changes. --- go/cmd/dolt/commands/add.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index d6d9b85edb..8a0d48d6dc 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -17,14 +17,13 @@ package commands import ( "bytes" "context" - "io" - "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/utils/argparser" + "github.com/dolthub/go-mysql-server/sql" ) var addDocs = cli.CommandDocumentationContent{ @@ -119,21 +118,16 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE } } - _, rowIter, err := queryist.Query(sqlCtx, generateSql(apr)) + schema, rowIter, err := queryist.Query(sqlCtx, generateSql(apr)) if err != nil { cli.PrintErrln(errhand.VerboseErrorFromError(err)) return 1 } - for { - _, err := rowIter.Next(sqlCtx) - if err == io.EOF { - return 0 - } - if err != nil { - cli.PrintErrln(errhand.VerboseErrorFromError(err)) - return 1 - } + _, err = sql.RowIterToRows(sqlCtx, schema, rowIter) + if err != nil { + cli.PrintErrln(errhand.VerboseErrorFromError(err)) + return 1 } return 0 From 1536a298d43ce9762ddec44b93e068439ff98d19 Mon Sep 17 00:00:00 2001 From: nicktobey Date: Wed, 24 May 2023 23:00:14 +0000 Subject: [PATCH 12/64] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/add.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 8a0d48d6dc..f44d0cf0e6 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -17,13 +17,15 @@ package commands import ( "bytes" "context" + + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/utils/argparser" - "github.com/dolthub/go-mysql-server/sql" ) var addDocs = cli.CommandDocumentationContent{ From 8fe476d7621daf17311d40bc3adb36d029193506 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Wed, 24 May 2023 17:12:29 -0700 Subject: [PATCH 13/64] Add test that `dolt add` functions as expected when server is running. --- integration-tests/bats/sql-local-remote.bats | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 98461b36fc..ad565e1022 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -21,6 +21,14 @@ teardown() { teardown_common } +get_staged_tables() { + dolt status | awk ' + match($0, /new table:\ */) { print substr($0, RSTART+RLENGTH) } + /Untracked tables:/ { exit } + /Tables with conflicting dolt_ignore patterns:/ { exit } + ' +} + @test "sql-local-remote: test switch between server/no server" { start_sql_server defaultDB @@ -108,3 +116,19 @@ teardown() { [[ "$output" =~ "defaultDB does not exist" ]] || false } +@test "sql-local-remote: verify simple dolt add behavior." { + start_sql_server altDb + ROOT_DIR=$(pwd) + + mkdir -p someplace_new/fun + cd someplace_new/fun + + dolt --verbose-engine-setup --data-dir="$ROOT_DIR/altDB" --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" + dolt --verbose-engine-setup --data-dir="$ROOT_DIR/altDB" --user dolt add . + + stop_sql_server 1 + + staged=$(get_staged_tables) + + [[ ! -z $(echo "$staged" | grep "testtable") ]] || false +} \ No newline at end of file From 88d047b312cfb1389367312d7a993810bbe76d4a Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Thu, 25 May 2023 09:31:34 -0700 Subject: [PATCH 14/64] Rename `subCommandsWithDEnv` to `commandsWithoutCliCtx` since some commands use both. --- go/cmd/dolt/dolt.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 179ba71dc8..d839d4a098 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -119,7 +119,7 @@ var doltSubCommands = []cli.Command{ &commands.Assist{}, } -var subCommandsUsingDEnv = []cli.Command{ +var commandsWithoutCliCtx = []cli.Command{ commands.InitCmd{}, commands.StatusCmd{}, commands.DiffCmd{}, @@ -135,7 +135,6 @@ var subCommandsUsingDEnv = []cli.Command{ commands.CheckoutCmd{}, commands.MergeCmd{}, cnfcmds.Commands, - commands.CherryPickCmd{}, commands.RevertCmd{}, commands.CloneCmd{}, commands.FetchCmd{}, @@ -169,7 +168,7 @@ var subCommandsUsingDEnv = []cli.Command{ } func initCliContext(commandName string) bool { - for _, command := range subCommandsUsingDEnv { + for _, command := range commandsWithoutCliCtx { if command.Name() == commandName { return false } From 5227ec4361c9e73360b61a2d322850d844015c44 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Thu, 25 May 2023 09:31:54 -0700 Subject: [PATCH 15/64] `dolt cherry pick` should forward CliCtx to `AddCmd` --- go/cmd/dolt/commands/cherry-pick.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 75ae000b7f..c435b1e796 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -97,12 +97,12 @@ func (cmd CherryPickCmd) Exec(ctx context.Context, commandStr string, args []str return HandleVErrAndExitCode(verr, usage) } - verr := cherryPick(ctx, dEnv, cherryStr) + verr := cherryPick(ctx, dEnv, cliCtx, cherryStr) return HandleVErrAndExitCode(verr, usage) } // cherryPick returns error if any step of cherry-picking fails. It receives cherry-picked commit and performs cherry-picking and commits. -func cherryPick(ctx context.Context, dEnv *env.DoltEnv, cherryStr string) errhand.VerboseError { +func cherryPick(ctx context.Context, dEnv *env.DoltEnv, cliCtx cli.CliContext, cherryStr string) errhand.VerboseError { // check for clean working state headRoot, err := dEnv.HeadRoot(ctx) if err != nil { @@ -156,13 +156,13 @@ func cherryPick(ctx context.Context, dEnv *env.DoltEnv, cherryStr string) errhan if err != nil { return errhand.VerboseErrorFromError(err) } - res := AddCmd{}.Exec(ctx, "add", []string{"-A"}, dEnv, nil) + res := AddCmd{}.Exec(ctx, "add", []string{"-A"}, dEnv, cliCtx) if res != 0 { return errhand.BuildDError("dolt add failed").AddCause(err).Build() } commitParams := []string{"-m", commitMsg} - res = CommitCmd{}.Exec(ctx, "commit", commitParams, dEnv, nil) + res = CommitCmd{}.Exec(ctx, "commit", commitParams, dEnv, cliCtx) if res != 0 { return errhand.BuildDError("dolt commit failed").AddCause(err).Build() } From b4a1685f4d91f207c8cfbad52c3c68659d967d96 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Tue, 30 May 2023 11:02:27 -0700 Subject: [PATCH 16/64] Encode all relevant information in the DoltIgnoreConflictError in its error message so it is propagated through remote connections. --- go/cmd/dolt/commands/add.go | 13 +------------ go/libraries/doltcore/doltdb/errors.go | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index f44d0cf0e6..58001a8799 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -165,18 +165,7 @@ func toStageVErr(err error) errhand.VerboseError { return bdr.Build() case doltdb.AsDoltIgnoreInConflict(err) != nil: - doltIgnoreConflictError := doltdb.AsDoltIgnoreInConflict(err) - bdr := errhand.BuildDError("error: the table %s matches conflicting patterns in dolt_ignore", doltIgnoreConflictError.Table) - - for _, pattern := range doltIgnoreConflictError.TruePatterns { - bdr.AddDetails("ignored: %s", pattern) - } - - for _, pattern := range doltIgnoreConflictError.FalsePatterns { - bdr.AddDetails("not ignored: %s", pattern) - } - - return bdr.Build() + return errhand.VerboseErrorFromError(err) default: return errhand.BuildDError("Unknown error").AddCause(err).Build() } diff --git a/go/libraries/doltcore/doltdb/errors.go b/go/libraries/doltcore/doltdb/errors.go index c09f42b118..c21be0624c 100644 --- a/go/libraries/doltcore/doltdb/errors.go +++ b/go/libraries/doltcore/doltdb/errors.go @@ -15,6 +15,7 @@ package doltdb import ( + "bytes" "errors" "fmt" ) @@ -180,7 +181,22 @@ type DoltIgnoreConflictError struct { } func (dc DoltIgnoreConflictError) Error() string { - return fmt.Sprintf("dolt_ignore has multiple conflicting rules for %s", dc.Table) + var buffer bytes.Buffer + buffer.WriteString("the table ") + buffer.WriteString(dc.Table) + buffer.WriteString(" matches conflicting patterns in dolt_ignore:") + + for _, pattern := range dc.TruePatterns { + buffer.WriteString("\nignored: ") + buffer.WriteString(pattern) + } + + for _, pattern := range dc.FalsePatterns { + buffer.WriteString("\nnot ignored: ") + buffer.WriteString(pattern) + } + + return buffer.String() } func AsDoltIgnoreInConflict(err error) *DoltIgnoreConflictError { From 0c1dc62a3ab24e96e28eb5fff7e2bc86b4a97d80 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Tue, 30 May 2023 13:47:30 -0700 Subject: [PATCH 17/64] Remove unneeded data-dir param from add tests. --- integration-tests/bats/sql-local-remote.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index ad565e1022..b73536023d 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -123,8 +123,8 @@ get_staged_tables() { mkdir -p someplace_new/fun cd someplace_new/fun - dolt --verbose-engine-setup --data-dir="$ROOT_DIR/altDB" --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" - dolt --verbose-engine-setup --data-dir="$ROOT_DIR/altDB" --user dolt add . + dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" + dolt --verbose-engine-setup --user dolt add . stop_sql_server 1 From e5435e31523137e906f94f99ddcc2a958ac667bd Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Tue, 30 May 2023 14:14:38 -0700 Subject: [PATCH 18/64] In sql-local-remote test for dolt add, verify that commands are running in remote mode. --- integration-tests/bats/sql-local-remote.bats | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index b73536023d..0db7148741 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -123,8 +123,13 @@ get_staged_tables() { mkdir -p someplace_new/fun cd someplace_new/fun - dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" - dolt --verbose-engine-setup --user dolt add . + run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" + [ "$status" -eq 0 ] + [[ "$output" =~ "starting remote mode" ]] || false + + run dolt --verbose-engine-setup --user dolt add . + [ "$status" -eq 0 ] + [[ "$output" =~ "starting remote mode" ]] || false stop_sql_server 1 From 8fabc2e545a9558e6eb96f18b0a3565f53891768 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Tue, 30 May 2023 14:52:19 -0700 Subject: [PATCH 19/64] sql-local-remote: cd into correct directory for add test. --- integration-tests/bats/sql-local-remote.bats | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 0db7148741..42626377b1 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -118,10 +118,7 @@ get_staged_tables() { @test "sql-local-remote: verify simple dolt add behavior." { start_sql_server altDb - ROOT_DIR=$(pwd) - - mkdir -p someplace_new/fun - cd someplace_new/fun + cd altDb run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" [ "$status" -eq 0 ] From 3287ad8b00d17b09f0252c7195c388b16ac9e8c9 Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Tue, 30 May 2023 16:49:01 -0700 Subject: [PATCH 20/64] updates dolt blame to use appropriate sql engine for results --- go/cmd/dolt/commands/blame.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 2105f128fd..545d631cfa 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -19,6 +19,7 @@ import ( "fmt" "github.com/dolthub/dolt/go/cmd/dolt/cli" + "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/utils/argparser" @@ -88,7 +89,23 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, usage() return 1 } - args = []string{"--" + QueryFlag, fmt.Sprintf(blameQueryTemplate, apr.Arg(0))} - return SqlCmd{}.Exec(ctx, "sql", args, dEnv, cliCtx) + queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) + if err != nil { + return 1 + } + if closeFunc != nil { + defer closeFunc() + } + + schema, ri, err := queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0))) + if err != nil { + return 1 + } + err = engine.PrettyPrintResults(sqlCtx, engine.FormatTabular, schema, ri) + if err != nil { + return 1 + } + + return 0 } From a305308d3f708ad932ebc8e99d55be5aa3c6627d Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Tue, 30 May 2023 17:03:51 -0700 Subject: [PATCH 21/64] updates dolt blame to support revision specification --- go/cmd/dolt/commands/blame.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 545d631cfa..648298a10e 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -23,10 +23,12 @@ import ( eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/utils/argparser" + "github.com/dolthub/go-mysql-server/sql" ) const ( - blameQueryTemplate = "SELECT * FROM dolt_blame_%s" + blameQueryTemplate = "SELECT * FROM dolt_blame_%s" + blameAsOfQueryTemplate = "SELECT * FROM dolt_blame_%s AS OF %s" ) var blameDocs = cli.CommandDocumentationContent{ @@ -55,7 +57,7 @@ func (cmd BlameCmd) Docs() *cli.CommandDocumentation { } func (cmd BlameCmd) ArgParser() *argparser.ArgParser { - ap := argparser.NewArgParserWithMaxArgs(cmd.Name(), 1) + ap := argparser.NewArgParserWithMaxArgs(cmd.Name(), 2) return ap } @@ -85,7 +87,7 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, blameDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) - if apr.NArg() != 1 { + if apr.NArg() > 2 || apr.NArg() == 0 { usage() return 1 } @@ -98,10 +100,17 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, defer closeFunc() } - schema, ri, err := queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0))) + var schema sql.Schema + var ri sql.RowIter + if apr.NArg() == 1 { + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0))) + } else { + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameAsOfQueryTemplate, apr.Arg(1), apr.Arg(0))) + } if err != nil { return 1 } + err = engine.PrettyPrintResults(sqlCtx, engine.FormatTabular, schema, ri) if err != nil { return 1 From bd22c227a6d29cf548ec1f54cf8c2c8aacb95279 Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Wed, 31 May 2023 11:14:42 -0700 Subject: [PATCH 22/64] update existing dolt blame bats tests --- go/cmd/dolt/commands/blame.go | 2 +- integration-tests/bats/blame.bats | 44 +++++++++++++++---------------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 648298a10e..6d77d500cc 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -28,7 +28,7 @@ import ( const ( blameQueryTemplate = "SELECT * FROM dolt_blame_%s" - blameAsOfQueryTemplate = "SELECT * FROM dolt_blame_%s AS OF %s" + blameAsOfQueryTemplate = "SELECT * FROM dolt_blame_%s AS OF '%s'" ) var blameDocs = cli.CommandDocumentationContent{ diff --git a/integration-tests/bats/blame.bats b/integration-tests/bats/blame.bats index 2bcc91233f..e7df5f9fda 100644 --- a/integration-tests/bats/blame.bats +++ b/integration-tests/bats/blame.bats @@ -50,55 +50,53 @@ SQL [[ "$output" =~ "usage" ]] || false } +@test "blame: too many arguments shows usage" { + run dolt blame HEAD blame_test foo + [ "$status" -eq 1 ] + [[ "$output" =~ "blame has too many positional arguments" ]] || false +} + @test "blame: annotates a small table with simple history" { # should be the same as dolt blame HEAD blame_test run dolt blame blame_test [ "$status" -eq 0 ] - # TODO: Make these assertions better - [[ "$output" =~ "Thomas Foolery" ]] || false - [[ "$output" =~ "create blame_test table" ]] || false + [[ "$output" =~ "1".*"Thomas Foolery".*"create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "Harry Wombat" ]] || false - [[ "$output" =~ "replace richard" ]] || false - [[ "$output" =~ "Johnny Moolah" ]] || false - [[ "$output" =~ "add more people" ]] || false + [[ "$output" =~ "2".*"Harry Wombat".*"replace richard" ]] || false + [[ "$output" =~ "3".*"Johnny Moolah".*"add more people" ]] || false + [[ "$output" =~ "4".*"Johnny Moolah".*"add more people" ]] || false } @test "blame: blames HEAD when commit ref omitted" { run dolt blame blame_test [ "$status" -eq 0 ] - [[ "$output" =~ "Thomas Foolery" ]] || false - [[ "$output" =~ "create blame_test table" ]] || false + + [[ "$output" =~ "1".*"Thomas Foolery".*"create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "Harry Wombat" ]] || false - [[ "$output" =~ "replace richard" ]] || false - [[ "$output" =~ "Johnny Moolah" ]] || false - [[ "$output" =~ "add more people" ]] || false + [[ "$output" =~ "2".*"Harry Wombat".*"replace richard" ]] || false + [[ "$output" =~ "3".*"Johnny Moolah".*"add more people" ]] || false + [[ "$output" =~ "4".*"Johnny Moolah".*"add more people" ]] || false } @test "blame: works with HEAD as the commit ref" { - skip "SQL views do no support AS OF queries" - run dolt blame HEAD blame_test [ "$status" -eq 0 ] - [[ "$output" =~ "Thomas Foolery" ]] || false - [[ "$output" =~ "create blame_test table" ]] || false + + [[ "$output" =~ "1".*"Thomas Foolery".*"create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "Harry Wombat" ]] || false - [[ "$output" =~ "replace richard" ]] || false - [[ "$output" =~ "Johnny Moolah" ]] || false - [[ "$output" =~ "add more people" ]] || false + [[ "$output" =~ "2".*"Harry Wombat".*"replace richard" ]] || false + [[ "$output" =~ "3".*"Johnny Moolah".*"add more people" ]] || false + [[ "$output" =~ "4".*"Johnny Moolah".*"add more people" ]] || false } @test "blame: works with HEAD~1 as the commit ref" { - skip "SQL views do no support AS OF queries" - run dolt blame HEAD~1 blame_test [ "$status" -eq 0 ] + [[ "$output" =~ "Thomas Foolery" ]] || false [[ "$output" =~ "create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false From c4eb8e05b01c2fe51eb6f376dab15acbf2372272 Mon Sep 17 00:00:00 2001 From: stephkyou Date: Wed, 31 May 2023 18:28:33 +0000 Subject: [PATCH 23/64] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/blame.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 6d77d500cc..15219462a6 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -18,12 +18,13 @@ import ( "context" "fmt" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/utils/argparser" - "github.com/dolthub/go-mysql-server/sql" ) const ( From 413bc12da05cadb7270e1a7bcb551f892c000eea Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Wed, 31 May 2023 12:16:36 -0700 Subject: [PATCH 24/64] minor dolt blame updates for cleanliness and more tests --- go/cmd/dolt/commands/blame.go | 7 +++---- integration-tests/bats/blame.bats | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 15219462a6..d8427519db 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -28,8 +28,7 @@ import ( ) const ( - blameQueryTemplate = "SELECT * FROM dolt_blame_%s" - blameAsOfQueryTemplate = "SELECT * FROM dolt_blame_%s AS OF '%s'" + blameQueryTemplate = "SELECT * FROM dolt_blame_%s AS OF '%s'" ) var blameDocs = cli.CommandDocumentationContent{ @@ -104,9 +103,9 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, var schema sql.Schema var ri sql.RowIter if apr.NArg() == 1 { - schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0))) + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0), "HEAD")) } else { - schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameAsOfQueryTemplate, apr.Arg(1), apr.Arg(0))) + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(1), apr.Arg(0))) } if err != nil { return 1 diff --git a/integration-tests/bats/blame.bats b/integration-tests/bats/blame.bats index e7df5f9fda..e468732699 100644 --- a/integration-tests/bats/blame.bats +++ b/integration-tests/bats/blame.bats @@ -123,10 +123,9 @@ SQL } @test "blame: works with HEAD~3 as the commit ref" { - skip "SQL views do no support AS OF queries" - run dolt blame HEAD~3 blame_test [ "$status" -eq 0 ] + [[ "$output" =~ "Thomas Foolery" ]] || false [[ "$output" =~ "create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false From 76c4b6cdafbe9b94083bc15a08a2f4e2769c7751 Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Wed, 31 May 2023 17:31:42 -0700 Subject: [PATCH 25/64] add remote test for dolt blame --- integration-tests/bats/blame.bats | 5 ++-- integration-tests/bats/sql-local-remote.bats | 26 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/integration-tests/bats/blame.bats b/integration-tests/bats/blame.bats index e468732699..b92e6066c1 100644 --- a/integration-tests/bats/blame.bats +++ b/integration-tests/bats/blame.bats @@ -137,10 +137,9 @@ SQL } @test "blame: returns an error when the table is not found in the given revision" { - skip "SQL views do no support AS OF queries" run dolt blame HEAD~4 blame_test - [ "$status" -eq 1 ] - [[ "$output" =~ "no table named blame_test found" ]] || false + [ "$status" -eq 0 ] + [[ "$output" = "" ]] || false } @test "blame: pk ordered output" { diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 98461b36fc..ad6b17c209 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -108,3 +108,29 @@ teardown() { [[ "$output" =~ "defaultDB does not exist" ]] || false } +@test "sql-local-remote: verify dolt blame behavior is identical in switch between server/no server" { + cd defaultDB + + dolt sql -q "create table test (pk int primary key)" + dolt sql -q "insert into test values (1)" + dolt add test + dolt commit -m "insert initial value into test" + dolt sql -q "insert into test values (2), (3)" + dolt add test + dolt commit -m "insert more values into test" + + start_sql_server defaultDB + run dolt --user dolt blame test + [ "$status" -eq 0 ] + [[ "$output" =~ "1".*"insert initial value into test" ]] || false + [[ "$output" =~ "2".*"insert more values into test" ]] || false + [[ "$output" =~ "3".*"insert more values into test" ]] || false + stop_sql_server 1 + + run dolt blame test + [ "$status" -eq 0 ] + [[ "$output" =~ "1".*"insert initial value into test" ]] || false + [[ "$output" =~ "2".*"insert more values into test" ]] || false + [[ "$output" =~ "3".*"insert more values into test" ]] || false +} + From 4a4bd88ffb67077d55ef0d6806f3f8b6a07f8984 Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Wed, 31 May 2023 17:48:47 -0700 Subject: [PATCH 26/64] clarify skip message in blame.bats --- integration-tests/bats/blame.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/blame.bats b/integration-tests/bats/blame.bats index b92e6066c1..6962b1f0c6 100644 --- a/integration-tests/bats/blame.bats +++ b/integration-tests/bats/blame.bats @@ -108,7 +108,7 @@ SQL } @test "blame: works with HEAD~2 as the commit ref" { - skip "SQL views do no support AS OF queries" + skip "SQL views return incorrect data when using AS OF with commits that modify existing data" run dolt blame HEAD~2 blame_test [ "$status" -eq 0 ] From 5260397aed2e516f71a9437cd9ffa43e95509694 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Thu, 1 Jun 2023 11:40:45 -0700 Subject: [PATCH 27/64] Add basic tests for `dolt add` --- integration-tests/bats/add.bats | 70 +++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 integration-tests/bats/add.bats diff --git a/integration-tests/bats/add.bats b/integration-tests/bats/add.bats new file mode 100644 index 0000000000..25bda04b75 --- /dev/null +++ b/integration-tests/bats/add.bats @@ -0,0 +1,70 @@ +#! /usr/bin/env bats +load $BATS_TEST_DIRNAME/helper/common.bash + +setup() { + setup_common +} + +teardown() { + teardown_common +} + +get_staged_tables() { + dolt status | awk ' + match($0, /new table:\ */) { print substr($0, RSTART+RLENGTH) } + /Untracked tables:/ { exit } + /Tables with conflicting dolt_ignore patterns:/ { exit } + ' +} + +get_working_tables() { + dolt status | awk ' + BEGIN { working = 0 } + (working == 1) && match($0, /new table:\ */) { print substr($0, RSTART+RLENGTH) } + /Untracked tables:/ { working = 1 } + /Tables with conflicting dolt_ignore patterns:/ { working = 0 } + ' +} + +@test "add: add dot" { + + dolt sql -q "create table testtable (pk int PRIMARY KEY)" + + staged=$(get_staged_tables) + working=$(get_working_tables) + + [[ ! -z $(echo "$working" | grep "testtable") ]] || false + [[ -z $(echo "$staged" | grep "testtable") ]] || false + + dolt add . + + staged=$(get_staged_tables) + working=$(get_working_tables) + + [[ ! -z $(echo "$staged" | grep "testtable") ]] || false + [[ -z $(echo "$working" | grep "testtable") ]] || false +} + +@test "add: add by name." { + + dolt sql -q "create table addedTable (pk int PRIMARY KEY)" + dolt sql -q "create table notAddedTable (pk int PRIMARY KEY)" + + staged=$(get_staged_tables) + working=$(get_working_tables) + + [[ ! -z $(echo "$working" | grep "addedTable") ]] || false + [[ -z $(echo "$staged" | grep "addedTable") ]] || false + [[ ! -z $(echo "$working" | grep "notAddedTable") ]] || false + [[ -z $(echo "$staged" | grep "notAddedTable") ]] || false + + dolt add addedTable + + staged=$(get_staged_tables) + working=$(get_working_tables) + + [[ ! -z $(echo "$staged" | grep "addedTable") ]] || false + [[ -z $(echo "$working" | grep "addedTable") ]] || false + [[ ! -z $(echo "$working" | grep "notAddedTable") ]] || false + [[ -z $(echo "$staged" | grep "notAddedTable") ]] || false +} \ No newline at end of file From 492ec12bfe19ed146e8b685aa7edd3dffa8d34ca Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Thu, 1 Jun 2023 13:45:55 -0700 Subject: [PATCH 28/64] Adding merge support for evaluating column default expressions that contain column references and functions --- .../doltcore/merge/merge_prolly_rows.go | 51 +++++++++---------- .../sqle/enginetest/dolt_queries_merge.go | 47 +++++++++++++---- 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go index ebc50f3236..9470b4de95 100644 --- a/go/libraries/doltcore/merge/merge_prolly_rows.go +++ b/go/libraries/doltcore/merge/merge_prolly_rows.go @@ -25,7 +25,6 @@ import ( "github.com/dolthub/go-mysql-server/memory" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/analyzer" - "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/planbuilder" "github.com/dolthub/go-mysql-server/sql/transform" errorkinds "gopkg.in/src-d/go-errors.v1" @@ -405,7 +404,6 @@ func (cv checkValidator) validateDiff(ctx *sql.Context, diff tree.ThreeWayDiff) var valueTuple val.Tuple var valueDesc val.TupleDesc - switch diff.Op { case tree.DiffOpLeftDelete, tree.DiffOpRightDelete, tree.DiffOpConvergentDelete: // no need to validate check constraints for deletes @@ -439,7 +437,7 @@ func (cv checkValidator) validateDiff(ctx *sql.Context, diff tree.ThreeWayDiff) newTuple = val.NewTuple(cv.valueMerger.syncPool, newTupleBytes...) } - row, err := cv.buildRow(ctx, diff.Key, newTuple) + row, err := buildRow(ctx, diff.Key, newTuple, cv.sch, cv.tableMerger) if err != nil { return 0, err } @@ -482,10 +480,10 @@ func (cv checkValidator) insertArtifact(ctx context.Context, key, value val.Tupl } // buildRow takes the |key| and |value| tuple and returns a new sql.Row, along with any errors encountered. -func (cv checkValidator) buildRow(ctx *sql.Context, key, value val.Tuple) (sql.Row, error) { - pkCols := cv.sch.GetPKCols() - valueCols := cv.sch.GetNonPKCols() - allCols := cv.sch.GetAllCols() +func buildRow(ctx *sql.Context, key, value val.Tuple, sch schema.Schema, tableMerger *TableMerger) (sql.Row, error) { + pkCols := sch.GetPKCols() + valueCols := sch.GetNonPKCols() + allCols := sch.GetAllCols() // When we parse and resolve the check constraint expression with planbuilder, it leaves row position 0 // for the expression itself, so we add an empty spot in index 0 of our row to account for that to make sure @@ -494,10 +492,10 @@ func (cv checkValidator) buildRow(ctx *sql.Context, key, value val.Tuple) (sql.R // Skip adding the key tuple if we're working with a keyless table, since the table row data is // always all contained in the value tuple for keyless tables. - if !cv.valueMerger.keyless { - keyDesc := cv.sch.GetKeyDescriptor() + if !schema.IsKeyless(sch) { + keyDesc := sch.GetKeyDescriptor() for i := range keyDesc.Types { - value, err := index.GetField(ctx, keyDesc, i, key, cv.tableMerger.ns) + value, err := index.GetField(ctx, keyDesc, i, key, tableMerger.ns) if err != nil { return nil, err } @@ -508,15 +506,15 @@ func (cv checkValidator) buildRow(ctx *sql.Context, key, value val.Tuple) (sql.R } valueColIndex := 0 - valueDescriptor := cv.sch.GetValueDescriptor() + valueDescriptor := sch.GetValueDescriptor() for valueTupleIndex := range valueDescriptor.Types { // Skip processing the first value in the value tuple for keyless tables, since that field // always holds the cardinality of the row and shouldn't be passed in to an expression. - if cv.valueMerger.keyless && valueTupleIndex == 0 { + if schema.IsKeyless(sch) && valueTupleIndex == 0 { continue } - value, err := index.GetField(ctx, valueDescriptor, valueTupleIndex, value, cv.tableMerger.ns) + value, err := index.GetField(ctx, valueDescriptor, valueTupleIndex, value, tableMerger.ns) if err != nil { return nil, err } @@ -932,7 +930,7 @@ func (m *primaryMerger) merge(ctx *sql.Context, diff tree.ThreeWayDiff, sourceSc return fmt.Errorf("cannot merge keyless tables with reordered columns") } } else { - tempTupleValue, err := remapTupleWithColumnDefaults(ctx, diff.Right, sourceSch.GetValueDescriptor(), + tempTupleValue, err := remapTupleWithColumnDefaults(ctx, diff.Key, diff.Right, sourceSch.GetValueDescriptor(), m.valueMerger.rightMapping, m.tableMerger, m.finalSch, m.valueMerger.syncPool) if err != nil { return err @@ -1103,7 +1101,7 @@ func remapTuple(tuple val.Tuple, desc val.TupleDesc, mapping val.OrdinalMapping) // currently being merged and associated node store. |mergedSch| is the new schema of the table and is used to look up // column default values to apply to any existing rows when a new column is added as part of a merge. |pool| is used to // to allocate memory for the new tuple. A pointer to the new tuple data is returned, along with any error encountered. -func remapTupleWithColumnDefaults(ctx *sql.Context, tuple val.Tuple, tupleDesc val.TupleDesc, mapping val.OrdinalMapping, tm *TableMerger, mergedSch schema.Schema, pool pool.BuffPool) (val.Tuple, error) { +func remapTupleWithColumnDefaults(ctx *sql.Context, keyTuple, valueTuple val.Tuple, tupleDesc val.TupleDesc, mapping val.OrdinalMapping, tm *TableMerger, mergedSch schema.Schema, pool pool.BuffPool) (val.Tuple, error) { tb := val.NewTupleBuilder(mergedSch.GetValueDescriptor()) for to, from := range mapping { @@ -1113,22 +1111,21 @@ func remapTupleWithColumnDefaults(ctx *sql.Context, tuple val.Tuple, tupleDesc v col := mergedSch.GetNonPKCols().GetByIndex(to) if col.Default != "" { // TODO: Not great to reparse the expression for every single row... need to cache this - defaultValue, err := parse.StringToColumnDefaultValue(ctx, col.Default) + expression, err := resolveExpression(ctx, col.Default, mergedSch, tm.name) if err != nil { return nil, err } - // TODO: We can currently only handle column default values that only use literal - // values. Any expressions that need to be resolved (e.g. column references, - // functions) need the analyzer invoked on them before we can Eval() them. - // So, instead of potentially creating inconsistent data and not applying the - // correct default value that customers are expecting, throw an error and alert - // customers that they need to manually apply the alter to update existing rows. - if !defaultValue.Expression.Resolved() { - return nil, ErrUnableToMergeColumnDefaultValue.New(defaultValue, tm.name) + if !expression.Resolved() { + return nil, ErrUnableToMergeColumnDefaultValue.New(col.Default, tm.name) } - value, err = defaultValue.Expression.Eval(ctx, nil) + row, err := buildRow(ctx, keyTuple, valueTuple, mergedSch, tm) + if err != nil { + return nil, err + } + + value, err = expression.Eval(ctx, row) if err != nil { return nil, err } @@ -1142,7 +1139,7 @@ func remapTupleWithColumnDefaults(ctx *sql.Context, tuple val.Tuple, tupleDesc v } } } else { - tb.PutRaw(to, tupleDesc.GetField(from, tuple)) + tb.PutRaw(to, tupleDesc.GetField(from, valueTuple)) } } return tb.Build(pool), nil @@ -1284,7 +1281,7 @@ func migrateDataToMergedSchema(ctx *sql.Context, tm *TableMerger, vm *valueMerge return err } - newValueTuple, err := remapTupleWithColumnDefaults(ctx, valueTuple, valueDescriptor, vm.leftMapping, tm, mergedSch, vm.syncPool) + newValueTuple, err := remapTupleWithColumnDefaults(ctx, keyTuple, valueTuple, valueDescriptor, vm.leftMapping, tm, mergedSch, vm.syncPool) if err != nil { return err } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 2e9c5fa9dd..9027c5f7b2 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4019,30 +4019,55 @@ var ThreeWayMergeWithSchemaChangeTestScripts = []MergeScriptTest{ }, }, { - // TODO: We can currently only support literal default values. Supporting column references and functions - // requires getting the analyzer involved to resolve references. Name: "adding a column with a non-literal default value", AncSetUpScript: []string{ - "CREATE table t (pk int primary key);", - "INSERT into t values (1);", + "CREATE table t (pk varchar(100) primary key);", + "INSERT into t values ('1');", }, RightSetUpScript: []string{ - "alter table t add column c1 varchar(100) default (CONCAT('h','e','l','l','o'));", - "insert into t values (2, 'hi');", + "alter table t add column c1 varchar(100) default (CONCAT(pk, 'h','e','l','l','o'));", + "insert into t values ('2', 'hi');", "alter table t add index idx1 (c1, pk);", }, LeftSetUpScript: []string{ - "insert into t values (3);", + "insert into t values ('3');", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "call dolt_merge('right');", - ExpectedErr: merge.ErrUnableToMergeColumnDefaultValue, + Query: "call dolt_merge('right');", + Expected: []sql.Row{{0, 0}}, }, { - Skip: true, Query: "select * from t;", - Expected: []sql.Row{{1, "hello"}, {2, "hi"}, {3, "hello"}}, + Expected: []sql.Row{{"1", "1hello"}, {"2", "hi"}, {"3", "3hello"}}, + }, + }, + }, + { + // Tests that column default expressions are correctly evaluated when the left-side schema + // has changed and the right row needs to be mapped to the new left-side schema + Name: "right-side adds a column with a default value, left-side drops a column", + AncSetUpScript: []string{ + "CREATE table t (pk int primary key, c1 varchar(100), c2 varchar(100));", + "INSERT into t values ('1', 'BAD', 'hello');", + }, + RightSetUpScript: []string{ + "alter table t add column c3 varchar(100) default (CONCAT(c2, 'h','e','l','l','o'));", + "insert into t values ('2', 'BAD', 'hello', 'hi');", + "alter table t add index idx1 (c1, pk);", + }, + LeftSetUpScript: []string{ + "insert into t values ('3', 'BAD', 'hello');", + "alter table t drop column c1;", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('right');", + Expected: []sql.Row{{0, 0}}, + }, + { + Query: "select * from t;", + Expected: []sql.Row{{1, "hello", "hellohello"}, {2, "hello", "hi"}, {3, "hello", "hellohello"}}, }, }, }, From fc4d7439cd1f7a20647f4d10a6d963e68caa5ced Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Thu, 1 Jun 2023 16:34:40 -0700 Subject: [PATCH 29/64] update local and remote tests for dolt blame to be more accurate --- integration-tests/bats/blame.bats | 9 +++++---- integration-tests/bats/sql-local-remote.bats | 8 ++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/integration-tests/bats/blame.bats b/integration-tests/bats/blame.bats index 6962b1f0c6..70320875be 100644 --- a/integration-tests/bats/blame.bats +++ b/integration-tests/bats/blame.bats @@ -61,12 +61,13 @@ SQL run dolt blame blame_test [ "$status" -eq 0 ] - [[ "$output" =~ "1".*"Thomas Foolery".*"create blame_test table" ]] || false + [[ "${lines[1]}" =~ "pk".*"commit".*"commit_date".*"committer".*"email".*"message" ]] || false + [[ "$output" =~ "1 |".+"|".+"| Thomas Foolery, | bats-1@email.fake | create blame_test table |" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "2".*"Harry Wombat".*"replace richard" ]] || false - [[ "$output" =~ "3".*"Johnny Moolah".*"add more people" ]] || false - [[ "$output" =~ "4".*"Johnny Moolah".*"add more people" ]] || false + [[ "$output" =~ "2 |".+"|".+"| Harry Wombat, | bats-3@email.fake | replace richard with harry |" ]] || false + [[ "$output" =~ "3 |".+"|".+"| Johnny Moolah, | bats-4@email.fake | add more people to blame_test |" ]] || false + [[ "$output" =~ "4 |".+"|".+"| Johnny Moolah, | bats-4@email.fake | add more people to blame_test |" ]] || false } @test "blame: blames HEAD when commit ref omitted" { diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index ad6b17c209..3b3e50a774 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -122,15 +122,11 @@ teardown() { start_sql_server defaultDB run dolt --user dolt blame test [ "$status" -eq 0 ] - [[ "$output" =~ "1".*"insert initial value into test" ]] || false - [[ "$output" =~ "2".*"insert more values into test" ]] || false - [[ "$output" =~ "3".*"insert more values into test" ]] || false + export out="$output" stop_sql_server 1 run dolt blame test [ "$status" -eq 0 ] - [[ "$output" =~ "1".*"insert initial value into test" ]] || false - [[ "$output" =~ "2".*"insert more values into test" ]] || false - [[ "$output" =~ "3".*"insert more values into test" ]] || false + [[ "$output" = $out ]] || false } From b8ad54f7e580029e337b179a413bc0b0a6c5c421 Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Thu, 1 Jun 2023 17:39:07 -0700 Subject: [PATCH 30/64] add validation for ref spec and more detailed error messaging in dolt blame --- go/cmd/dolt/commands/blame.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index d8427519db..0b5ecf4f7d 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -18,6 +18,7 @@ import ( "context" "fmt" + "github.com/dolthub/dolt/go/libraries/utils/iohelp" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -94,6 +95,7 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) return 1 } if closeFunc != nil { @@ -105,14 +107,23 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, if apr.NArg() == 1 { schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0), "HEAD")) } else { + // validate input + _, err = ResolveCommitWithVErr(dEnv, apr.Arg(0)) + if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) + return 1 + } + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(1), apr.Arg(0))) } if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) return 1 } err = engine.PrettyPrintResults(sqlCtx, engine.FormatTabular, schema, ri) if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) return 1 } From 62fff578abb06f739bdcabe712140ca8c4cf5a34 Mon Sep 17 00:00:00 2001 From: stephkyou Date: Fri, 2 Jun 2023 00:51:42 +0000 Subject: [PATCH 31/64] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/blame.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 0b5ecf4f7d..55259872b3 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "github.com/dolthub/dolt/go/libraries/utils/iohelp" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -26,6 +25,7 @@ import ( eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/utils/argparser" + "github.com/dolthub/dolt/go/libraries/utils/iohelp" ) const ( From bef50ffebd153fa20ae88fb776a96e7d2c395f59 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Thu, 1 Jun 2023 19:24:04 -0700 Subject: [PATCH 32/64] Allow staging tables with merge conflicts. --- go/cmd/dolt/commands/add.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 58001a8799..608e500803 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -110,6 +110,13 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE defer closeFunc() } + // Allow staging tables with merge conflicts. + _, _, err = queryist.Query(sqlCtx, "set @@dolt_force_transaction_commit=1;") + if err != nil { + cli.PrintErrln(errhand.VerboseErrorFromError(err)) + return 1 + } + ap := cli.CreateAddArgParser() helpPr, _ := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, addDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, helpPr) From b32c5e6fede68aeb88c7b0830933adb25cec43bb Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Fri, 2 Jun 2023 09:45:59 -0700 Subject: [PATCH 33/64] updates dolt blame to work outside of dolt repos --- go/cmd/dolt/commands/blame.go | 6 ++++++ integration-tests/bats/sql-local-remote.bats | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 55259872b3..763eb73ddf 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -62,6 +62,12 @@ func (cmd BlameCmd) ArgParser() *argparser.ArgParser { return ap } +func (cmd BlameCmd) RequiresRepo() bool { + return false +} + +var _ cli.RepoNotRequiredCommand = BlameCmd{} + // EventType returns the type of the event to log func (cmd BlameCmd) EventType() eventsapi.ClientEventType { return eventsapi.ClientEventType_BLAME diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 3b3e50a774..db707e557e 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -109,8 +109,7 @@ teardown() { } @test "sql-local-remote: verify dolt blame behavior is identical in switch between server/no server" { - cd defaultDB - + cd altDB dolt sql -q "create table test (pk int primary key)" dolt sql -q "insert into test values (1)" dolt add test @@ -118,8 +117,9 @@ teardown() { dolt sql -q "insert into test values (2), (3)" dolt add test dolt commit -m "insert more values into test" + cd .. - start_sql_server defaultDB + start_sql_server altDB run dolt --user dolt blame test [ "$status" -eq 0 ] export out="$output" From f285c5bbe2a51b1b80b2e8eeef8849bc25b8341b Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Fri, 2 Jun 2023 12:02:12 -0700 Subject: [PATCH 34/64] update ref checking in dolt blame --- go/cmd/dolt/commands/blame.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 763eb73ddf..1806c216c7 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -17,7 +17,10 @@ package commands import ( "context" "fmt" + "regexp" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + ref2 "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -114,9 +117,9 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0), "HEAD")) } else { // validate input - _, err = ResolveCommitWithVErr(dEnv, apr.Arg(0)) - if err != nil { - iohelp.WriteLine(cli.CliOut, err.Error()) + ref := apr.Arg(0) + if !ref2.IsValidTagName(ref) && !doltdb.IsValidCommitHash(ref) && !isValidHeadRef(ref) { + iohelp.WriteLine(cli.CliOut, "Invalid reference provided") return 1 } @@ -135,3 +138,8 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, return 0 } + +func isValidHeadRef(s string) bool { + var refRegex = regexp.MustCompile(`(?i)^head[\~\^0-9]*$`) + return refRegex.MatchString(s) +} From 694ea98a0e20e37803cd0f8bb0657778c25ddea7 Mon Sep 17 00:00:00 2001 From: stephkyou Date: Fri, 2 Jun 2023 19:12:08 +0000 Subject: [PATCH 35/64] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/blame.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 1806c216c7..67dc94a360 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -19,14 +19,14 @@ import ( "fmt" "regexp" - "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" - ref2 "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" + ref2 "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/libraries/utils/iohelp" ) From d5e3d075beaa27f86c8b3c9315530d2675c2714e Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 12:21:53 -0700 Subject: [PATCH 36/64] Fail early when running CliContext-using-commands outside of a repo, with an exception for specific commands. --- go/cmd/dolt/dolt.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index d839d4a098..08990048f1 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -529,7 +529,7 @@ The sql subcommand is currently the only command that uses these flags. All othe var cliCtx cli.CliContext = nil if initCliContext(subcommandName) { - lateBind, err := buildLateBinder(ctx, dEnv.FS, mrEnv, apr, verboseEngineSetup) + lateBind, err := buildLateBinder(ctx, dEnv.FS, mrEnv, apr, subcommandName, verboseEngineSetup) if err != nil { cli.PrintErrln(color.RedString("Failure to Load SQL Engine: %v", err)) return 1 @@ -562,7 +562,7 @@ The sql subcommand is currently the only command that uses these flags. All othe return res } -func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.MultiRepoEnv, apr *argparser.ArgParseResults, verbose bool) (cli.LateBindQueryist, error) { +func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.MultiRepoEnv, apr *argparser.ArgParseResults, subcommandName string, verbose bool) (cli.LateBindQueryist, error) { var targetEnv *env.DoltEnv = nil @@ -580,6 +580,13 @@ func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.Mult targetEnv = mrEnv.GetEnv(useDb) } + // There is no target environment detected. This is allowed only for a small number of commands. + // We don't expect that number to grow, so we list them here. + isDoltEnvironmentRequired := subcommandName != "init" && subcommandName != "sql" && subcommandName != "sql-server" + if targetEnv == nil && isDoltEnvironmentRequired { + return nil, fmt.Errorf("The current directory is not a valid dolt repository.") + } + // nil targetEnv will happen if the user ran a command in an empty directory - which we support in some cases. if targetEnv != nil { isLocked, lock, err := targetEnv.GetLock() From 98b9a858b8860501d00ca952b66f28cbc5f5ea89 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Fri, 2 Jun 2023 12:49:48 -0700 Subject: [PATCH 37/64] Adding support for `--skip-empty` for `dolt commit` and `dolt_commit()` --- go/cmd/dolt/cli/arg_parser_helpers.go | 12 ++++ go/cmd/dolt/commands/commit.go | 65 ++++++++++++------- go/cmd/dolt/commands/merge.go | 4 +- go/libraries/doltcore/env/actions/commit.go | 5 ++ .../sqle/dprocedures/dolt_cherry_pick.go | 3 +- .../doltcore/sqle/dprocedures/dolt_commit.go | 61 ++++++++++------- .../doltcore/sqle/dprocedures/dolt_merge.go | 2 +- .../doltcore/sqle/dprocedures/dolt_revert.go | 2 +- integration-tests/bats/commit.bats | 21 ++++++ integration-tests/bats/helper/common.bash | 5 ++ integration-tests/bats/sql-commit.bats | 19 +++++- 11 files changed, 147 insertions(+), 52 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 6257a230e0..971ca64c18 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -77,6 +77,7 @@ func ParseAuthor(authorStr string) (string, string, error) { const ( AllowEmptyFlag = "allow-empty" + SkipEmptyFlag = "skip-empty" DateParam = "date" MessageArg = "message" AuthorParam = "author" @@ -141,6 +142,7 @@ func CreateCommitArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithMaxArgs("commit", 0) ap.SupportsString(MessageArg, "m", "msg", "Use the given {{.LessThan}}msg{{.GreaterThan}} as the commit message.") ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety.") + ap.SupportsFlag(SkipEmptyFlag, "", "Only create a commit if there are staged changes. If no changes are staged, the call to commit is a no-op.") ap.SupportsString(DateParam, "", "date", "Specify the date used in the commit. If not specified the current system time is used.") ap.SupportsFlag(ForceFlag, "f", "Ignores any foreign key warnings and proceeds with the commit.") ap.SupportsString(AuthorParam, "", "author", "Specify an explicit author using the standard A U Thor {{.LessThan}}author@example.com{{.GreaterThan}} format.") @@ -403,3 +405,13 @@ func VerifyNoAwsParams(apr *argparser.ArgParseResults) error { return nil } + +// VerifyCommitArgs validates the arguments in |apr| for `dolt commit` and returns an error +// if any validation problems were encountered. +func VerifyCommitArgs(apr *argparser.ArgParseResults) error { + if apr.Contains(AllowEmptyFlag) && apr.Contains(SkipEmptyFlag) { + return fmt.Errorf("error: cannot use both --allow-empty and --skip-empty") + } + + return nil +} diff --git a/go/cmd/dolt/commands/commit.go b/go/cmd/dolt/commands/commit.go index 7cf9cf1ae9..ad468cb0a4 100644 --- a/go/cmd/dolt/commands/commit.go +++ b/go/cmd/dolt/commands/commit.go @@ -77,41 +77,53 @@ func (cmd CommitCmd) ArgParser() *argparser.ArgParser { // Exec executes the command func (cmd CommitCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int { - res := performCommit(ctx, commandStr, args, dEnv) + res, skipped := performCommit(ctx, commandStr, args, dEnv) if res == 1 { return res } + if skipped { + iohelp.WriteLine(cli.CliOut, "Skipping empty commit") + return res + } + // if the commit was successful, print it out using the log command return LogCmd{}.Exec(ctx, "log", []string{"-n=1"}, dEnv, nil) } -func performCommit(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv) int { +// performCommit creates a new Dolt commit using the specified |commandStr| and |args| for the specified Dolt environment +// |dEnv|. The response is an integer status code indicating success or failure, as well as a boolean that indicates +// if the commit was skipped (e.g. because --skip-empty was specified as an argument). +func performCommit(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv) (int, bool) { ap := cli.CreateCommitArgParser() help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, commitDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) + if err := cli.VerifyCommitArgs(apr); err != nil { + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), help), false + } + allFlag := apr.Contains(cli.AllFlag) upperCaseAllFlag := apr.Contains(cli.UpperCaseAllFlag) if dEnv.IsLocked() { - return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help) + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help), false } roots, err := dEnv.Roots(ctx) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working root").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working root").AddCause(err).Build(), usage), false } if upperCaseAllFlag { roots, err = actions.StageAllTables(ctx, roots, true) if err != nil { - return handleCommitErr(ctx, dEnv, err, help) + return handleCommitErr(ctx, dEnv, err, help), false } } else if allFlag { roots, err = actions.StageModifiedAndDeletedTables(ctx, roots) if err != nil { - return handleCommitErr(ctx, dEnv, err, help) + return handleCommitErr(ctx, dEnv, err, help), false } } @@ -125,13 +137,13 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * } else { // This command creates a commit, so we need user identity if !cli.CheckUserNameAndEmail(dEnv) { - return 1 + return 1, false } name, email, err = env.GetNameAndEmail(dEnv.Config) } if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } msg, msgOk := apr.GetValue(cli.MessageArg) @@ -140,13 +152,13 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { commitMeta, cmErr := headCommit.GetCommitMeta(ctx) if cmErr != nil { - return handleCommitErr(ctx, dEnv, cmErr, usage) + return handleCommitErr(ctx, dEnv, cmErr, usage), false } amendStr = commitMeta.Description } msg, err = getCommitMessageFromEditor(ctx, dEnv, "", amendStr, false) if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } } @@ -156,7 +168,7 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * t, err = cli.ParseDate(commitTimeStr) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("error: invalid date").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("error: invalid date").AddCause(err).Build(), usage), false } } @@ -172,23 +184,23 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * newRoots, err := actions.ResetSoftToRef(ctx, dEnv.DbData(), "HEAD~1") if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } ws, err := dEnv.WorkingSet(ctx) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage), false } prevHash, err := ws.HashOf() if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage), false } var mergeParentCommits []*doltdb.Commit @@ -202,6 +214,7 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * Message: msg, Date: t, AllowEmpty: apr.Contains(cli.AllowEmptyFlag) || apr.Contains(cli.AmendFlag), + SkipEmpty: apr.Contains(cli.SkipEmptyFlag), Force: apr.Contains(cli.ForceFlag), Name: name, Email: email, @@ -210,20 +223,26 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { newRoots, errRes := actions.ResetSoftToRef(ctx, dEnv.DbData(), headHash.String()) if errRes != nil { - return handleResetError(errRes, usage) + return handleResetError(errRes, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false + } + + // If no error was reported and there is no pending commit, then no commit was created, likely + // because of --skip-empty, so return a success code, but indicate that no commit was created. + if pendingCommit == nil { + return 0, true } headRef, err := dEnv.RepoStateReader().CWBHeadRef() if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } _, err = dEnv.DoltDB.CommitWithWorkingSet( ctx, @@ -239,18 +258,18 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { newRoots, errRes := actions.ResetSoftToRef(ctx, dEnv.DbData(), headHash.String()) if errRes != nil { - return handleResetError(errRes, usage) + return handleResetError(errRes, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't commit").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't commit").AddCause(err).Build(), usage), false } - return 0 + return 0, false } func handleCommitErr(ctx context.Context, dEnv *env.DoltEnv, err error, usage cli.UsagePrinter) int { diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index 93651de708..bee68abe10 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -580,8 +580,8 @@ func executeMergeAndCommit(ctx context.Context, dEnv *env.DoltEnv, spec *merge.M author := fmt.Sprintf("%s <%s>", spec.Name, spec.Email) - res := performCommit(ctx, "commit", []string{"-m", msg, "--author", author}, dEnv) - if res != 0 { + res, skipped := performCommit(ctx, "commit", []string{"-m", msg, "--author", author}, dEnv) + if res != 0 || skipped { return nil, fmt.Errorf("dolt commit failed after merging") } diff --git a/go/libraries/doltcore/env/actions/commit.go b/go/libraries/doltcore/env/actions/commit.go index f348f71dc3..3fda0d3d9b 100644 --- a/go/libraries/doltcore/env/actions/commit.go +++ b/go/libraries/doltcore/env/actions/commit.go @@ -27,6 +27,7 @@ type CommitStagedProps struct { Message string Date time.Time AllowEmpty bool + SkipEmpty bool Amend bool Force bool Name string @@ -62,6 +63,10 @@ func GetCommitStaged( isEmpty := len(staged) == 0 allowEmpty := ws.MergeActive() || props.AllowEmpty || props.Amend + + if isEmpty && props.SkipEmpty { + return nil, nil + } if isEmpty && !allowEmpty { return nil, NothingStaged{notStaged} } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go index c73d138f4c..b5d691b638 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go @@ -93,7 +93,8 @@ func doDoltCherryPick(ctx *sql.Context, args []string) (string, error) { return "", fmt.Errorf("dolt add failed") } - return doDoltCommit(ctx, []string{"-m", commitMsg}) + commitHash, _, err := doDoltCommit(ctx, []string{"-m", commitMsg}) + return commitHash, err } // cherryPick checks that the current working set is clean, verifies the cherry-pick commit is not a merge commit diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go b/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go index 6699687743..72fd7825cd 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go @@ -32,51 +32,65 @@ var hashType = types.MustCreateString(query.Type_TEXT, 32, sql.Collation_ascii_b // doltCommit is the stored procedure version for the CLI command `dolt commit`. func doltCommit(ctx *sql.Context, args ...string) (sql.RowIter, error) { - res, err := doDoltCommit(ctx, args) + commitHash, skipped, err := doDoltCommit(ctx, args) if err != nil { return nil, err } - return rowToIter(res), nil + if skipped { + return nil, nil + } + return rowToIter(commitHash), nil } // doltCommitHashOut is the stored procedure version for the CLI function `commit`. The first parameter is the variable // to set the hash of. func doltCommitHashOut(ctx *sql.Context, outHash *string, args ...string) (sql.RowIter, error) { - res, err := doDoltCommit(ctx, args) + commitHash, skipped, err := doDoltCommit(ctx, args) if err != nil { return nil, err } - *outHash = res - return rowToIter(res), nil + if skipped { + return nil, nil + } + + *outHash = commitHash + return rowToIter(commitHash), nil } -func doDoltCommit(ctx *sql.Context, args []string) (string, error) { +// doDoltCommit creates a dolt commit using the specified command line |args| provided. The response is the commit hash +// of the new commit (or the empty string if the commit was skipped), a boolean that indicates if creating the commit +// was skipped (e.g. due to --skip-empty), and an error describing any error encountered. +func doDoltCommit(ctx *sql.Context, args []string) (string, bool, error) { if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil { - return "", err + return "", false, err } // Get the information for the sql context. dbName := ctx.GetCurrentDatabase() apr, err := cli.CreateCommitArgParser().Parse(args) if err != nil { - return "", err + return "", false, err + } + + if err := cli.VerifyCommitArgs(apr); err != nil { + return "", false, err } dSess := dsess.DSessFromSess(ctx.Session) roots, ok := dSess.GetRoots(ctx, dbName) if !ok { - return "", fmt.Errorf("Could not load database %s", dbName) + return "", false, fmt.Errorf("Could not load database %s", dbName) } if apr.Contains(cli.UpperCaseAllFlag) { roots, err = actions.StageAllTables(ctx, roots, true) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } else if apr.Contains(cli.AllFlag) { roots, err = actions.StageModifiedAndDeletedTables(ctx, roots) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } @@ -84,7 +98,7 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { name, email, err = cli.ParseAuthor(authorStr) if err != nil { - return "", err + return "", false, err } } else { // In SQL mode, use the current SQL user as the commit author, instead of the `dolt config` configured values. @@ -100,15 +114,15 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { if amend { commit, err := dSess.GetHeadCommit(ctx, dbName) if err != nil { - return "", err + return "", false, err } commitMeta, err := commit.GetCommitMeta(ctx) if err != nil { - return "", err + return "", false, err } msg = commitMeta.Description } else { - return "", fmt.Errorf("Must provide commit message.") + return "", false, fmt.Errorf("Must provide commit message.") } } @@ -118,7 +132,7 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { t, err = cli.ParseDate(commitTimeStr) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } @@ -126,31 +140,34 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { Message: msg, Date: t, AllowEmpty: apr.Contains(cli.AllowEmptyFlag), + SkipEmpty: apr.Contains(cli.SkipEmptyFlag), Amend: amend, Force: apr.Contains(cli.ForceFlag), Name: name, Email: email, }) if err != nil { - return "", err + return "", false, err } // Nothing to commit, and we didn't pass --allowEmpty - if pendingCommit == nil { - return "", errors.New("nothing to commit") + if pendingCommit == nil && apr.Contains(cli.SkipEmptyFlag) { + return "", true, nil + } else if pendingCommit == nil { + return "", false, errors.New("nothing to commit") } newCommit, err := dSess.DoltCommit(ctx, dbName, dSess.GetTransaction(), pendingCommit) if err != nil { - return "", err + return "", false, err } h, err := newCommit.HashOf() if err != nil { - return "", err + return "", false, err } - return h.String(), nil + return h.String(), false, nil } func getDoltArgs(ctx *sql.Context, row sql.Row, children []sql.Expression) ([]string, error) { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index a587de2328..b619fe1a13 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -222,7 +222,7 @@ func performMerge(ctx *sql.Context, sess *dsess.DoltSession, roots doltdb.Roots, if !noCommit { author := fmt.Sprintf("%s <%s>", spec.Name, spec.Email) - _, err = doDoltCommit(ctx, []string{"-m", msg, "--author", author}) + _, _, err = doDoltCommit(ctx, []string{"-m", msg, "--author", author}) if err != nil { return ws, noConflictsOrViolations, threeWayMerge, fmt.Errorf("dolt_commit failed") } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go b/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go index f930a6fb82..4124fdee75 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go @@ -128,7 +128,7 @@ func doDoltRevert(ctx *sql.Context, args []string) (int, error) { if err != nil { return 1, err } - _, err = doDoltCommit(ctx, commitArgs) + _, _, err = doDoltCommit(ctx, commitArgs) if err != nil { return 1, err } diff --git a/integration-tests/bats/commit.bats b/integration-tests/bats/commit.bats index 2f94b3669f..917c9bc295 100644 --- a/integration-tests/bats/commit.bats +++ b/integration-tests/bats/commit.bats @@ -51,4 +51,25 @@ teardown() { run dolt commit [ $status -eq 1 ] [[ "$output" =~ "Failed to open commit editor" ]] || false +} + +@test "commit: --skip-empty correctly skips committing when no changes are staged" { + dolt sql -q "create table t(pk int primary key);" + dolt add t + original_head=$(get_head_commit) + + # When --allow-empty and --skip-empty are both specified, the user should get an error + run dolt commit --allow-empty --skip-empty -m "commit message" + [ $status -eq 1 ] + [[ "$output" =~ 'error: cannot use both --allow-empty and --skip-empty' ]] || false + [ $original_head = $(get_head_commit) ] + + # When changes are staged, --skip-empty has no effect + dolt commit --skip-empty -m "commit message" + new_head=$(get_head_commit) + [ $original_head != $new_head ] + + # When no changes are staged, --skip-empty skips creating the commit + dolt commit --skip-empty -m "commit message" + [ $new_head = $(get_head_commit) ] } \ No newline at end of file diff --git a/integration-tests/bats/helper/common.bash b/integration-tests/bats/helper/common.bash index 7e85640e3e..c252f2ecce 100644 --- a/integration-tests/bats/helper/common.bash +++ b/integration-tests/bats/helper/common.bash @@ -38,6 +38,11 @@ current_dolt_user_email() { dolt config --global --get user.email } +# get_head_commit returns the commit hash for the current HEAD +get_head_commit() { + dolt log -n 1 | grep -m 1 commit | cut -c 13-44 +} + setup_no_dolt_init() { export PATH=$PATH:~/go/bin cd $BATS_TMPDIR diff --git a/integration-tests/bats/sql-commit.bats b/integration-tests/bats/sql-commit.bats index 80503fd257..78eb38f871 100644 --- a/integration-tests/bats/sql-commit.bats +++ b/integration-tests/bats/sql-commit.bats @@ -453,6 +453,21 @@ SQL [[ "$output" =~ 'error: no value for option `message' ]] || false } -get_head_commit() { - dolt log -n 1 | grep -m 1 commit | cut -c 13-44 +@test "sql-commit: --skip-empty correctly skips committing when no changes are staged" { + original_head=$(get_head_commit) + + # When --allow-empty and --skip-empty are both specified, the user should get an error + run dolt sql -q "CALL DOLT_COMMIT('--allow-empty', '--skip-empty', '-m', 'commit message');" + [ $status -eq 1 ] + [[ "$output" =~ 'error: cannot use both --allow-empty and --skip-empty' ]] || false + [ $original_head = $(get_head_commit) ] + + # When changes are staged, --skip-empty has no effect + dolt sql -q "CALL DOLT_COMMIT('--skip-empty', '-m', 'commit message');" + new_head=$(get_head_commit) + [ $original_head != $new_head ] + + # When no changes are staged, --skip-empty skips creating the commit + dolt sql -q "CALL DOLT_COMMIT('--skip-empty', '-m', 'commit message');" + [ $new_head = $(get_head_commit) ] } From 0b135017b0d61860460ec684a168e7631bf2444d Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Fri, 2 Jun 2023 12:53:54 -0700 Subject: [PATCH 38/64] Adding documentation that --skip-empty cannot be used together with --allow-empty --- go/cmd/dolt/cli/arg_parser_helpers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 971ca64c18..bd13ef2ee8 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -141,8 +141,8 @@ var branchForceFlagDesc = "Reset {{.LessThan}}branchname{{.GreaterThan}} to {{.L func CreateCommitArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithMaxArgs("commit", 0) ap.SupportsString(MessageArg, "m", "msg", "Use the given {{.LessThan}}msg{{.GreaterThan}} as the commit message.") - ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety.") - ap.SupportsFlag(SkipEmptyFlag, "", "Only create a commit if there are staged changes. If no changes are staged, the call to commit is a no-op.") + ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety. Cannot be used with --skip-empty.") + ap.SupportsFlag(SkipEmptyFlag, "", "Only create a commit if there are staged changes. If no changes are staged, the call to commit is a no-op. Cannot be used with --allow-empty.") ap.SupportsString(DateParam, "", "date", "Specify the date used in the commit. If not specified the current system time is used.") ap.SupportsFlag(ForceFlag, "f", "Ignores any foreign key warnings and proceeds with the commit.") ap.SupportsString(AuthorParam, "", "author", "Specify an explicit author using the standard A U Thor {{.LessThan}}author@example.com{{.GreaterThan}} format.") From 54b8e0f4b330fe6861896aa3d77c737243dcd0f7 Mon Sep 17 00:00:00 2001 From: James Cor Date: Fri, 2 Jun 2023 13:46:31 -0700 Subject: [PATCH 39/64] adding more info to error and fixing tests --- go/libraries/doltcore/sqle/dsess/transactions.go | 6 +++++- .../doltcore/sqle/enginetest/dolt_transaction_queries.go | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/sqle/dsess/transactions.go b/go/libraries/doltcore/sqle/dsess/transactions.go index 9be838ed59..bced833c23 100644 --- a/go/libraries/doltcore/sqle/dsess/transactions.go +++ b/go/libraries/doltcore/sqle/dsess/transactions.go @@ -544,12 +544,16 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working // TODO: We need to add more granularity in terms of what types of constraint violations can be committed. For example, // in the case of foreign_key_checks=0 you should be able to commit foreign key violations. if forceTransactionCommit.(int8) != 1 { + badTbls, err := workingRoot.TablesWithConstraintViolations(ctx) + if err != nil { + return err + } rollbackErr := tx.rollback(ctx) if rollbackErr != nil { return rollbackErr } - return ErrUnresolvedConstraintViolationsCommit + return fmt.Errorf("%s Constraint violations in tables: %v", ErrUnresolvedConstraintViolationsCommit, badTbls) } } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go index 8759096130..ef1c4033c9 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go @@ -1995,7 +1995,8 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + + "Constraint violations in tables: [child]", }, { Query: "/* client b */ INSERT INTO child VALUES (1, 1);", @@ -2024,7 +2025,8 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + + "Constraint violations in tables: [t]", }, { Query: "/* client a */ SELECT * from DOLT_CONSTRAINT_VIOLATIONS;", @@ -2107,7 +2109,8 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + + "Constraint violations in tables: [child]", }, }, }, From 98f717e2363dae985dd70265214a244622cbc49e Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 13:52:07 -0700 Subject: [PATCH 40/64] Give `RevertCmd` a CliCtx so it can pass it to `AddCmd` --- go/cmd/dolt/commands/revert.go | 2 +- go/cmd/dolt/dolt.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/revert.go b/go/cmd/dolt/commands/revert.go index fbc388047e..6174004f85 100644 --- a/go/cmd/dolt/commands/revert.go +++ b/go/cmd/dolt/commands/revert.go @@ -144,7 +144,7 @@ func (cmd RevertCmd) Exec(ctx context.Context, commandStr string, args []string, if err != nil { return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } - res := AddCmd{}.Exec(ctx, "add", []string{"-A"}, dEnv, nil) + res := AddCmd{}.Exec(ctx, "add", []string{"-A"}, dEnv, cliCtx) if res != 0 { return res } diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 08990048f1..0442256eee 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -135,7 +135,6 @@ var commandsWithoutCliCtx = []cli.Command{ commands.CheckoutCmd{}, commands.MergeCmd{}, cnfcmds.Commands, - commands.RevertCmd{}, commands.CloneCmd{}, commands.FetchCmd{}, commands.PullCmd{}, From e3ea1e031bec75fbfce3f3bc6c7e12de53cfc429 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 15:18:13 -0700 Subject: [PATCH 41/64] Delay getting a Queryist until after parsing command line args, in case --help is passed. --- go/cmd/dolt/commands/add.go | 8 ++++---- go/cmd/dolt/dolt.go | 9 +++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index 608e500803..f7dd398fb3 100644 --- a/go/cmd/dolt/commands/add.go +++ b/go/cmd/dolt/commands/add.go @@ -101,6 +101,10 @@ func generateSql(apr *argparser.ArgParseResults) string { // Exec executes the command func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int { + ap := cli.CreateAddArgParser() + helpPr, _ := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, addDocs, ap)) + apr := cli.ParseArgsOrDie(ap, args, helpPr) + queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) if err != nil { cli.PrintErrln(errhand.VerboseErrorFromError(err)) @@ -117,10 +121,6 @@ func (cmd AddCmd) Exec(ctx context.Context, commandStr string, args []string, dE return 1 } - ap := cli.CreateAddArgParser() - helpPr, _ := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, addDocs, ap)) - apr := cli.ParseArgsOrDie(ap, args, helpPr) - for _, tableName := range apr.Args { if tableName != "." && !doltdb.IsValidTableName(tableName) { return HandleVErrAndExitCode(errhand.BuildDError("'%s' is not a valid table name", tableName).Build(), nil) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 0442256eee..05cfcac173 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -19,6 +19,7 @@ import ( crand "crypto/rand" "encoding/binary" "fmt" + "github.com/dolthub/go-mysql-server/sql" "math/rand" "net/http" _ "net/http/pprof" @@ -579,11 +580,15 @@ func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.Mult targetEnv = mrEnv.GetEnv(useDb) } - // There is no target environment detected. This is allowed only for a small number of commands. + // There is no target environment detected. This is allowed for a small number of command. // We don't expect that number to grow, so we list them here. + // It's also allowed when --help is passed. + // So we defer the error until the caller tries to use the cli.LateBindQueryist isDoltEnvironmentRequired := subcommandName != "init" && subcommandName != "sql" && subcommandName != "sql-server" if targetEnv == nil && isDoltEnvironmentRequired { - return nil, fmt.Errorf("The current directory is not a valid dolt repository.") + return func(ctx context.Context) (cli.Queryist, *sql.Context, func(), error) { + return nil, nil, nil, fmt.Errorf("The current directory is not a valid dolt repository.") + }, nil } // nil targetEnv will happen if the user ran a command in an empty directory - which we support in some cases. From 3935d9aff2070390c239464a45b327f47061c645 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 15:28:49 -0700 Subject: [PATCH 42/64] sql-local-remote.bats is for manually testing CLI commands in both local and remote mode. It shouldn't be included in the remote-server CI testing. --- integration-tests/bats/helper/local-remote.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/bats/helper/local-remote.bash b/integration-tests/bats/helper/local-remote.bash index 5cdeb0eb9b..970e1d5ffd 100644 --- a/integration-tests/bats/helper/local-remote.bash +++ b/integration-tests/bats/helper/local-remote.bash @@ -32,6 +32,7 @@ SKIP_SERVER_TESTS=$(cat <<-EOM ~sql-charsets-collations.bats~ ~sql-cherry-pick.bats~ ~sql-commit.bats~ +~sql-local-remote.bats~ ~primary-key-changes.bats~ ~common.bash.bats~ ~remotes-localbs.bats~ From 0d69f95604d2ca359bf816fffe4e3a99be3a5a88 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 17:46:59 -0700 Subject: [PATCH 43/64] Make exception in buildLateBinder for `dolt sql-client` --- go/cmd/dolt/dolt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 4f5af7a447..9a89098a54 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -584,7 +584,7 @@ func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.Mult // We don't expect that number to grow, so we list them here. // It's also allowed when --help is passed. // So we defer the error until the caller tries to use the cli.LateBindQueryist - isDoltEnvironmentRequired := subcommandName != "init" && subcommandName != "sql" && subcommandName != "sql-server" + isDoltEnvironmentRequired := subcommandName != "init" && subcommandName != "sql" && subcommandName != "sql-server" && subcommandName != "sql-client" if targetEnv == nil && isDoltEnvironmentRequired { return func(ctx context.Context) (cli.Queryist, *sql.Context, func(), error) { return nil, nil, nil, fmt.Errorf("The current directory is not a valid dolt repository.") From b1d1ea80de6b92575b286cfdf2edb7f49e3a91c8 Mon Sep 17 00:00:00 2001 From: James Cor Date: Sat, 3 Jun 2023 00:03:48 -0700 Subject: [PATCH 44/64] error --- .../doltcore/sqle/dsess/transactions.go | 88 ++++++++++++++++++- .../enginetest/dolt_transaction_queries.go | 6 +- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/sqle/dsess/transactions.go b/go/libraries/doltcore/sqle/dsess/transactions.go index bced833c23..165eb876a4 100644 --- a/go/libraries/doltcore/sqle/dsess/transactions.go +++ b/go/libraries/doltcore/sqle/dsess/transactions.go @@ -16,8 +16,11 @@ package dsess import ( "context" + "encoding/json" "errors" "fmt" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" + "github.com/dolthub/dolt/go/store/prolly" "strings" "sync" "time" @@ -548,12 +551,95 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } + + violations := make([]string, len(badTbls)) + for i, name := range badTbls { + tbl, _, err := workingRoot.GetTable(ctx, name) + if err != nil { + return err + } + + artIdx, err := tbl.GetArtifacts(ctx) + if err != nil { + return err + } + + m := durable.ProllyMapFromArtifactIndex(artIdx) + itr, err := m.IterAllCVs(ctx) + + for { + art, err := itr.Next(ctx) + if err != nil { + break + } + + var meta prolly.ConstraintViolationMeta + err = json.Unmarshal(art.Metadata, &meta) + if err != nil { + return err + } + + s := "" + switch art.ArtType { + case prolly.ArtifactTypeForeignKeyViol: + var m merge.FkCVMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + + s += fmt.Sprintf("Type: Foreign Key Constraint Violation\n" + + "ForeignKey: %s,\n" + + "Table: %s,\n" + + "ReferencedTable: %s,\n" + + "Index: %s,\n" + + "ReferencedIndex: %s\n", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) + + case prolly.ArtifactTypeUniqueKeyViol: + var m merge.UniqCVMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + s += fmt.Sprintf(": %s,\n" + + "Table: %s,\n" + + "ReferencedTable: %s,\n" + + "Index: %s,\n" + + "ReferencedIndex: %s\n", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) + + case prolly.ArtifactTypeNullViol: + var m merge.NullViolationMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + case prolly.ArtifactTypeChkConsViol: + var m merge.CheckCVMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + default: + panic("json not implemented for artifact type") + } + if err != nil { + return err + } + + for k, v := range res { + s += fmt.Sprintf("%s: %s\n", k, v) + } + + violations[i] = s + } + } + rollbackErr := tx.rollback(ctx) if rollbackErr != nil { return rollbackErr } - return fmt.Errorf("%s Constraint violations in tables: %v", ErrUnresolvedConstraintViolationsCommit, badTbls) + return fmt.Errorf("%s Constraint violations: %s", ErrUnresolvedConstraintViolationsCommit, strings.Join(violations, ", ")) } } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go index ef1c4033c9..502dcab19d 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go @@ -1996,7 +1996,7 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + - "Constraint violations in tables: [child]", + "Constraint violations: Columns: [parent_fk]\nOnDelete: RESTRICT\nReferencedColumns: [pk]\nReferencedIndex: \nTable: child\nForeignKey: 0050p5ek\nIndex: parent_fk\nOnUpdate: RESTRICT\nReferencedTable: parent\n", }, { Query: "/* client b */ INSERT INTO child VALUES (1, 1);", @@ -2026,7 +2026,7 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + - "Constraint violations in tables: [t]", + "Constraint violations: Columns: [col1]\nName: col1\n", }, { Query: "/* client a */ SELECT * from DOLT_CONSTRAINT_VIOLATIONS;", @@ -2110,7 +2110,7 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + - "Constraint violations in tables: [child]", + "Constraint violations: Table: child\nForeignKey: fk_name\nOnUpdate: RESTRICT\nReferencedTable: parent\nReferencedColumns: [v1]\nReferencedIndex: v1\nColumns: [v1]\nIndex: v1\nOnDelete: RESTRICT\n", }, }, }, From 27b848623cdbe2a885f78a73eb33b567e97e8291 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 19:53:41 -0700 Subject: [PATCH 45/64] Ensure we skip `sql-local-remote.bats` when running in remote server CI mode. --- integration-tests/bats/sql-local-remote.bats | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index db707e557e..9aefe6d8dd 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -11,6 +11,7 @@ make_repo() { } setup() { + setup_remote_server setup_no_dolt_init make_repo defaultDB make_repo altDB @@ -130,3 +131,22 @@ teardown() { [[ "$output" = $out ]] || false } +@test "sql-local-remote: verify simple dolt add behavior." { + start_sql_server altDb + + cd altDb + + run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" + [ "$status" -eq 0 ] + [[ "$output" =~ "starting remote mode" ]] || false + + run dolt --verbose-engine-setup --user dolt add . + [ "$status" -eq 0 ] + [[ "$output" =~ "starting remote mode" ]] || false + + stop_sql_server 1 + + staged=$(get_staged_tables) + + [[ ! -z $(echo "$staged" | grep "testtable") ]] || false +} \ No newline at end of file From 25969993627b1a4f3477339f8687b0cd297c5d24 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Sat, 3 Jun 2023 14:02:40 -0700 Subject: [PATCH 46/64] Fix case sensitivity issue in `sql-local-remote.bats` test. --- integration-tests/bats/sql-local-remote.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 9aefe6d8dd..6b3da5f1d2 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -132,9 +132,9 @@ teardown() { } @test "sql-local-remote: verify simple dolt add behavior." { - start_sql_server altDb + start_sql_server altDB - cd altDb + cd altDB run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" [ "$status" -eq 0 ] From af6c15a22194dfd5621e4f54ee54a9da76f5e888 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Sat, 3 Jun 2023 14:05:42 -0700 Subject: [PATCH 47/64] Make `sql-local-remote.bats` more explicit about skipping remote-engine tests. --- integration-tests/bats/sql-local-remote.bats | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 6b3da5f1d2..393c84f226 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -11,7 +11,9 @@ make_repo() { } setup() { - setup_remote_server + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "This test tests remote connections directly, SQL_ENGINE is not needed." + fi setup_no_dolt_init make_repo defaultDB make_repo altDB From c2f58bd406f8b65f0df4a7fb51e5db773edb82aa Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Sat, 3 Jun 2023 17:07:39 -0700 Subject: [PATCH 48/64] Use correct matrix variable when setting env variable in `ci-bats-unix.yaml` --- .github/workflows/ci-bats-unix.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-bats-unix.yaml b/.github/workflows/ci-bats-unix.yaml index d20f6f3e9b..0c9dd655d2 100644 --- a/.github/workflows/ci-bats-unix.yaml +++ b/.github/workflows/ci-bats-unix.yaml @@ -111,7 +111,7 @@ jobs: run: expect -v - name: Test all Unix env: - SQL_ENGINE: ${{ matrix.sql-engine }} + SQL_ENGINE: ${{ matrix.sql_engine }} PARQUET_RUNTIME_JAR: ${{ steps.parquet_cli.outputs.runtime_jar }} BATS_TEST_RETRIES: "3" run: | From 248fffdf08e9e8d69efef0e58968c3d4aab615ac Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Sat, 3 Jun 2023 20:16:59 -0700 Subject: [PATCH 49/64] Don't run `dolt config --unset` in tests if there are no vars to unset (because the test was skipped.) --- integration-tests/bats/replication-multidb.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/replication-multidb.bats b/integration-tests/bats/replication-multidb.bats index 7bc31d356a..8d3875c0d2 100644 --- a/integration-tests/bats/replication-multidb.bats +++ b/integration-tests/bats/replication-multidb.bats @@ -52,7 +52,7 @@ teardown() { cd $BATS_TMPDIR if ! [ "$DOLT_DEFAULT_BIN_FORMAT" = "__DOLT__" ]; then - dolt config --list | awk '{ print $1 }' | grep sqlserver.global | xargs dolt config --global --unset + dolt config --list | awk '{ print $1 }' | grep sqlserver.global | xargs --no-run-if-empty dolt config --global --unset fi } From 79912bfe6ace4ac3407db51ae7cdc5a1dba551b5 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Sat, 3 Jun 2023 23:39:01 -0700 Subject: [PATCH 50/64] `xargs --no-run-if-empty` does not exist on macOS, use `xargs -r` instead. --- integration-tests/bats/replication-multidb.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/replication-multidb.bats b/integration-tests/bats/replication-multidb.bats index 8d3875c0d2..04b01e4f90 100644 --- a/integration-tests/bats/replication-multidb.bats +++ b/integration-tests/bats/replication-multidb.bats @@ -52,7 +52,7 @@ teardown() { cd $BATS_TMPDIR if ! [ "$DOLT_DEFAULT_BIN_FORMAT" = "__DOLT__" ]; then - dolt config --list | awk '{ print $1 }' | grep sqlserver.global | xargs --no-run-if-empty dolt config --global --unset + dolt config --list | awk '{ print $1 }' | grep sqlserver.global | xargs -r dolt config --global --unset fi } From a0a90d911034727c4a1d3e1f58e5c20085b78b21 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 5 Jun 2023 09:30:35 -0700 Subject: [PATCH 51/64] Remove `dolt add` test. It will get added with the actual `dolt add ` migration PR. --- integration-tests/bats/sql-local-remote.bats | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 393c84f226..e7d5336482 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -132,23 +132,3 @@ teardown() { [ "$status" -eq 0 ] [[ "$output" = $out ]] || false } - -@test "sql-local-remote: verify simple dolt add behavior." { - start_sql_server altDB - - cd altDB - - run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" - [ "$status" -eq 0 ] - [[ "$output" =~ "starting remote mode" ]] || false - - run dolt --verbose-engine-setup --user dolt add . - [ "$status" -eq 0 ] - [[ "$output" =~ "starting remote mode" ]] || false - - stop_sql_server 1 - - staged=$(get_staged_tables) - - [[ ! -z $(echo "$staged" | grep "testtable") ]] || false -} \ No newline at end of file From be8056793e465d9379ac973b04341106c8ccfe18 Mon Sep 17 00:00:00 2001 From: nicktobey Date: Mon, 5 Jun 2023 18:22:38 +0000 Subject: [PATCH 52/64] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/dolt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 9a89098a54..024b393bad 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -19,7 +19,6 @@ import ( crand "crypto/rand" "encoding/binary" "fmt" - "github.com/dolthub/go-mysql-server/sql" "math/rand" "net/http" _ "net/http/pprof" @@ -28,6 +27,7 @@ import ( "strconv" "time" + "github.com/dolthub/go-mysql-server/sql" "github.com/fatih/color" "github.com/pkg/profile" "go.opentelemetry.io/otel" From f1503edfdeeec29c6e5429375a5f00afea98f2a0 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 5 Jun 2023 11:56:52 -0700 Subject: [PATCH 53/64] detailed and formatted error --- .../merge/violations_unique_prolly.go | 2 +- .../doltcore/sqle/dsess/transactions.go | 40 ++++++++++--------- .../enginetest/dolt_transaction_queries.go | 27 ++++++++++--- 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/go/libraries/doltcore/merge/violations_unique_prolly.go b/go/libraries/doltcore/merge/violations_unique_prolly.go index 0117c63ad1..0742b34ae1 100644 --- a/go/libraries/doltcore/merge/violations_unique_prolly.go +++ b/go/libraries/doltcore/merge/violations_unique_prolly.go @@ -163,7 +163,7 @@ func (m NullViolationMeta) ToString(ctx *sql.Context) (string, error) { // CheckCVMeta holds metadata describing a check constraint violation. type CheckCVMeta struct { Name string `json:"Name"` - Expression string `jason:"Expression"` + Expression string `json:"Expression"` } var _ types.JSONValue = CheckCVMeta{} diff --git a/go/libraries/doltcore/sqle/dsess/transactions.go b/go/libraries/doltcore/sqle/dsess/transactions.go index 165eb876a4..0b1080994f 100644 --- a/go/libraries/doltcore/sqle/dsess/transactions.go +++ b/go/libraries/doltcore/sqle/dsess/transactions.go @@ -587,13 +587,13 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } - - s += fmt.Sprintf("Type: Foreign Key Constraint Violation\n" + - "ForeignKey: %s,\n" + - "Table: %s,\n" + - "ReferencedTable: %s,\n" + - "Index: %s,\n" + - "ReferencedIndex: %s\n", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) + s = fmt.Sprintf("\n" + + "Type: Foreign Key Constraint Violation\n" + + "\tForeignKey: %s,\n" + + "\tTable: %s,\n" + + "\tReferencedTable: %s,\n" + + "\tIndex: %s,\n" + + "\tReferencedIndex: %s", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) case prolly.ArtifactTypeUniqueKeyViol: var m merge.UniqCVMeta @@ -601,11 +601,10 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } - s += fmt.Sprintf(": %s,\n" + - "Table: %s,\n" + - "ReferencedTable: %s,\n" + - "Index: %s,\n" + - "ReferencedIndex: %s\n", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) + s = fmt.Sprintf("\n" + + "Type: Unique Key Constraint Violation,\n" + + "\tName: %s,\n" + + "\tColumns: %v", m.Name, m.Columns) case prolly.ArtifactTypeNullViol: var m merge.NullViolationMeta @@ -613,23 +612,25 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } + s = fmt.Sprintf("\n" + + "Type: Null Constraint Violation,\n" + + "\tColumns: %v", m.Columns) + case prolly.ArtifactTypeChkConsViol: var m merge.CheckCVMeta err = json.Unmarshal(meta.VInfo, &m) if err != nil { return err } - default: - panic("json not implemented for artifact type") + s = fmt.Sprintf("\n" + + "Type: Check Constraint Violation,\n" + + "\tName: %s,\n" + + "\tExpression: %v", m.Name, m.Expression) } if err != nil { return err } - for k, v := range res { - s += fmt.Sprintf("%s: %s\n", k, v) - } - violations[i] = s } } @@ -639,7 +640,8 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working return rollbackErr } - return fmt.Errorf("%s Constraint violations: %s", ErrUnresolvedConstraintViolationsCommit, strings.Join(violations, ", ")) + return fmt.Errorf("%s\n" + + "Constraint violations: %s", ErrUnresolvedConstraintViolationsCommit, strings.Join(violations, ", ")) } } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go index 502dcab19d..ebd07816d6 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go @@ -1995,8 +1995,14 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + - "Constraint violations: Columns: [parent_fk]\nOnDelete: RESTRICT\nReferencedColumns: [pk]\nReferencedIndex: \nTable: child\nForeignKey: 0050p5ek\nIndex: parent_fk\nOnUpdate: RESTRICT\nReferencedTable: parent\n", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + + "Constraint violations: \n" + + "Type: Foreign Key Constraint Violation\n" + + "\tForeignKey: 0050p5ek,\n" + + "\tTable: child,\n" + + "\tReferencedTable: ,\n" + + "\tIndex: parent_fk,\n" + + "\tReferencedIndex: ", }, { Query: "/* client b */ INSERT INTO child VALUES (1, 1);", @@ -2025,8 +2031,11 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + - "Constraint violations: Columns: [col1]\nName: col1\n", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + + "Constraint violations: \n" + + "Type: Unique Key Constraint Violation,\n" + + "\tName: col1,\n" + + "\tColumns: [col1]", }, { Query: "/* client a */ SELECT * from DOLT_CONSTRAINT_VIOLATIONS;", @@ -2109,8 +2118,14 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1. " + - "Constraint violations: Table: child\nForeignKey: fk_name\nOnUpdate: RESTRICT\nReferencedTable: parent\nReferencedColumns: [v1]\nReferencedIndex: v1\nColumns: [v1]\nIndex: v1\nOnDelete: RESTRICT\n", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + + "Constraint violations: \n" + + "Type: Foreign Key Constraint Violation\n" + + "\tForeignKey: fk_name,\n" + + "\tTable: child,\n" + + "\tReferencedTable: v1,\n" + + "\tIndex: v1,\n" + + "\tReferencedIndex: v1", }, }, }, From 63e74b7a3cc3e2fdc9b6d0928ea85294ab86ef96 Mon Sep 17 00:00:00 2001 From: JCOR11599 Date: Mon, 5 Jun 2023 19:06:37 +0000 Subject: [PATCH 54/64] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- .../doltcore/sqle/dsess/transactions.go | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/go/libraries/doltcore/sqle/dsess/transactions.go b/go/libraries/doltcore/sqle/dsess/transactions.go index 0b1080994f..6c0c2986bf 100644 --- a/go/libraries/doltcore/sqle/dsess/transactions.go +++ b/go/libraries/doltcore/sqle/dsess/transactions.go @@ -19,8 +19,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" - "github.com/dolthub/dolt/go/store/prolly" "strings" "sync" "time" @@ -30,12 +28,14 @@ import ( "github.com/sirupsen/logrus" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/dolt/go/store/hash" + "github.com/dolthub/dolt/go/store/prolly" ) const ( @@ -587,12 +587,12 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } - s = fmt.Sprintf("\n" + - "Type: Foreign Key Constraint Violation\n" + - "\tForeignKey: %s,\n" + - "\tTable: %s,\n" + - "\tReferencedTable: %s,\n" + - "\tIndex: %s,\n" + + s = fmt.Sprintf("\n"+ + "Type: Foreign Key Constraint Violation\n"+ + "\tForeignKey: %s,\n"+ + "\tTable: %s,\n"+ + "\tReferencedTable: %s,\n"+ + "\tIndex: %s,\n"+ "\tReferencedIndex: %s", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) case prolly.ArtifactTypeUniqueKeyViol: @@ -601,9 +601,9 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } - s = fmt.Sprintf("\n" + - "Type: Unique Key Constraint Violation,\n" + - "\tName: %s,\n" + + s = fmt.Sprintf("\n"+ + "Type: Unique Key Constraint Violation,\n"+ + "\tName: %s,\n"+ "\tColumns: %v", m.Name, m.Columns) case prolly.ArtifactTypeNullViol: @@ -612,8 +612,8 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } - s = fmt.Sprintf("\n" + - "Type: Null Constraint Violation,\n" + + s = fmt.Sprintf("\n"+ + "Type: Null Constraint Violation,\n"+ "\tColumns: %v", m.Columns) case prolly.ArtifactTypeChkConsViol: @@ -622,9 +622,9 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working if err != nil { return err } - s = fmt.Sprintf("\n" + - "Type: Check Constraint Violation,\n" + - "\tName: %s,\n" + + s = fmt.Sprintf("\n"+ + "Type: Check Constraint Violation,\n"+ + "\tName: %s,\n"+ "\tExpression: %v", m.Name, m.Expression) } if err != nil { @@ -640,7 +640,7 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working return rollbackErr } - return fmt.Errorf("%s\n" + + return fmt.Errorf("%s\n"+ "Constraint violations: %s", ErrUnresolvedConstraintViolationsCommit, strings.Join(violations, ", ")) } } From 0d88a6aab8c360560a98d0343621752cbd44a05f Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 5 Jun 2023 12:28:14 -0700 Subject: [PATCH 55/64] Fix case sensitivity issue in `sql-local-remote.bats` --- integration-tests/bats/sql-local-remote.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index a25c38f3fa..b3ce627ffd 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -141,8 +141,8 @@ get_staged_tables() { [[ "$output" = $out ]] || false } @test "sql-local-remote: verify simple dolt add behavior." { - start_sql_server altDb - cd altDb + start_sql_server altDB + cd altDB run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)" [ "$status" -eq 0 ] From ed8f23481c3b862e32ca314d41cd966d29757bb1 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 5 Jun 2023 13:23:35 -0700 Subject: [PATCH 56/64] Integration tests for load data ignore/replace --- integration-tests/bats/sql-load-data.bats | 3 -- .../tests/sql-server-orig.yaml | 34 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/integration-tests/bats/sql-load-data.bats b/integration-tests/bats/sql-load-data.bats index a0e95d43f7..045b687023 100644 --- a/integration-tests/bats/sql-load-data.bats +++ b/integration-tests/bats/sql-load-data.bats @@ -294,7 +294,6 @@ CSV dolt sql -q "create table t (pk int primary key, c1 int, c2 int)" dolt sql -q "insert into t values (0,0,0)" - skip "load data ignore not supported" run dolt sql < Date: Mon, 5 Jun 2023 13:39:58 -0700 Subject: [PATCH 57/64] Fix go-sql-server-driver tests --- .../go-sql-server-driver/tests/sql-server-orig.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/go-sql-server-driver/tests/sql-server-orig.yaml b/integration-tests/go-sql-server-driver/tests/sql-server-orig.yaml index 388803541a..6a34140460 100644 --- a/integration-tests/go-sql-server-driver/tests/sql-server-orig.yaml +++ b/integration-tests/go-sql-server-driver/tests/sql-server-orig.yaml @@ -253,7 +253,7 @@ tests: - on: repo1 queries: - exec: "CREATE TABLE test(pk int primary key, c1 int, c2 int, c3 int, c4 int, c5 int)" - - exec: "INSERT INTO TABLE test VALUES ('0', '0', '0', '0', '0', '0')')" + - exec: "INSERT INTO test VALUES (0, 0, 0, 0, 0, 0)" - exec: "SET GLOBAL local_infile = 1" - exec: "LOAD DATA LOCAL INFILE 'testdata/1pk5col-ints.csv' INTO TABLE test CHARACTER SET UTF8MB4 FIELDS TERMINATED BY ',' ESCAPED BY '' LINES TERMINATED BY '\n' IGNORE 1 LINES" - query: "SELECT * FROM test" @@ -270,7 +270,7 @@ tests: - on: repo1 queries: - exec: "CREATE TABLE test(pk int primary key, c1 int, c2 int, c3 int, c4 int, c5 int)" - - exec: "INSERT INTO TABLE test VALUES ('0', '0', '0', '0', '0', '0')')" + - exec: "INSERT INTO test VALUES (0, 0, 0, 0, 0, 0)" - exec: "SET GLOBAL local_infile = 1" - exec: "LOAD DATA LOCAL INFILE 'testdata/1pk5col-ints.csv' REPLACE INTO TABLE test CHARACTER SET UTF8MB4 FIELDS TERMINATED BY ',' ESCAPED BY '' LINES TERMINATED BY '\n' IGNORE 1 LINES" - query: "SELECT * FROM test" From c6b6187220198ff601e837a4814a728907d23ae7 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 5 Jun 2023 15:52:34 -0700 Subject: [PATCH 58/64] Bump gms --- go/go.mod | 4 ++-- go/go.sum | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go/go.mod b/go/go.mod index eb44cfb9ba..ae2053f4fb 100644 --- a/go/go.mod +++ b/go/go.mod @@ -15,7 +15,7 @@ require ( github.com/dolthub/fslock v0.0.3 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 - github.com/dolthub/vitess v0.0.0-20230531182027-fe0363298c52 + github.com/dolthub/vitess v0.0.0-20230605213213-cbc62ca5fb39 github.com/dustin/go-humanize v1.0.0 github.com/fatih/color v1.13.0 github.com/flynn-archive/go-shlex v0.0.0-20150515145356-3f9db97f8568 @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f + github.com/dolthub/go-mysql-server v0.15.1-0.20230605224424-ada1bacfb2d3 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index 151d6002f5..fe1ecf62f5 100644 --- a/go/go.sum +++ b/go/go.sum @@ -170,6 +170,8 @@ github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f h1:f7H66wimomDcGtzBpTa7slxUIPUKY0t3nweGuS/GgQo= github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= +github.com/dolthub/go-mysql-server v0.15.1-0.20230605224424-ada1bacfb2d3 h1:/CHoQMpuL+Oy2r/Zy5k39GoDYZY29kyV/+L4bi4cS/g= +github.com/dolthub/go-mysql-server v0.15.1-0.20230605224424-ada1bacfb2d3/go.mod h1:E1STXoY8HvbrOWwFOhHpMfIjgcjboBhFtSpRxPgiBOw= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= @@ -182,6 +184,8 @@ github.com/dolthub/swiss v0.1.0 h1:EaGQct3AqeP/MjASHLiH6i4TAmgbG/c4rA6a1bzCOPc= github.com/dolthub/swiss v0.1.0/go.mod h1:BeucyB08Vb1G9tumVN3Vp/pyY4AMUnr9p7Rz7wJ7kAQ= github.com/dolthub/vitess v0.0.0-20230531182027-fe0363298c52 h1:MGNJQpIOGRxtcKEsmbuMXFhMIJdlfxeeBaDZjbI6ZAg= github.com/dolthub/vitess v0.0.0-20230531182027-fe0363298c52/go.mod h1:IyoysiiOzrIs7QsEHC+yVF0yRQ6W70GXyCXqtI2vVTs= +github.com/dolthub/vitess v0.0.0-20230605213213-cbc62ca5fb39 h1:T1oXYVAZrpANwuyStC/SZGw4cYmPFMwq6n6KvK4U6Mw= +github.com/dolthub/vitess v0.0.0-20230605213213-cbc62ca5fb39/go.mod h1:IyoysiiOzrIs7QsEHC+yVF0yRQ6W70GXyCXqtI2vVTs= github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= From cb7bdb4f32cdfc872c9d4b649c6a0e59b1414499 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 5 Jun 2023 18:38:27 -0700 Subject: [PATCH 59/64] Skip TestLoadDataErrors --- .../doltcore/sqle/enginetest/dolt_engine_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index e99278749f..5a74fbf1b6 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -991,6 +991,7 @@ func TestLoadData(t *testing.T) { } func TestLoadDataErrors(t *testing.T) { + t.Skip() h := newDoltHarness(t) defer h.Close() enginetest.TestLoadDataErrors(t, h) @@ -1559,10 +1560,10 @@ func TestSingleTransactionScript(t *testing.T) { Expected: // existing transaction logic []sql.Row{ - sql.Row{"j131v1r3cf6mrdjjjuqgkv4t33oa0l54", "billy bob", "bigbillieb@fake.horse", time.Date(1969, time.December, 31, 21, 0, 0, 0, time.Local), "Initialize data repository"}, - sql.Row{"kcg4345ir3tjfb13mr0on1bv1m56h9if", "billy bob", "bigbillieb@fake.horse", time.Date(1970, time.January, 1, 4, 0, 0, 0, time.Local), "checkpoint enginetest database mydb"}, - sql.Row{"9jtjpggd4t5nso3mefilbde3tkfosdna", "billy bob", "bigbillieb@fake.horse", time.Date(1970, time.January, 1, 12, 0, 0, 0, time.Local), "Step 1"}, - sql.Row{"559f6kdh0mm5i1o40hs3t8dr43bkerav", "billy bob", "bigbillieb@fake.horse", time.Date(1970, time.January, 2, 3, 0, 0, 0, time.Local), "update a value"}, + {"j131v1r3cf6mrdjjjuqgkv4t33oa0l54", "billy bob", "bigbillieb@fake.horse", time.Date(1969, time.December, 31, 21, 0, 0, 0, time.Local), "Initialize data repository"}, + {"kcg4345ir3tjfb13mr0on1bv1m56h9if", "billy bob", "bigbillieb@fake.horse", time.Date(1970, time.January, 1, 4, 0, 0, 0, time.Local), "checkpoint enginetest database mydb"}, + {"9jtjpggd4t5nso3mefilbde3tkfosdna", "billy bob", "bigbillieb@fake.horse", time.Date(1970, time.January, 1, 12, 0, 0, 0, time.Local), "Step 1"}, + {"559f6kdh0mm5i1o40hs3t8dr43bkerav", "billy bob", "bigbillieb@fake.horse", time.Date(1970, time.January, 2, 3, 0, 0, 0, time.Local), "update a value"}, }, // new tx logic From 0d292708780311b421258c555415a7c4413c969e Mon Sep 17 00:00:00 2001 From: Dustin Brown Date: Mon, 5 Jun 2023 21:49:54 -0700 Subject: [PATCH 60/64] [auto-bump] [no-release-notes] dependency by max-hoffman (#6091) * [ga-bump-dep] Bump dependency in Dolt by max-hoffman * collation test --------- Co-authored-by: max-hoffman Co-authored-by: Max Hoffman --- go/go.mod | 2 +- go/go.sum | 4 ++-- .../sqle/binlogreplication/binlog_replication_test.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go/go.mod b/go/go.mod index eb44cfb9ba..b0477f1929 100644 --- a/go/go.mod +++ b/go/go.mod @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f + github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index 151d6002f5..5499793233 100644 --- a/go/go.sum +++ b/go/go.sum @@ -168,8 +168,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f h1:f7H66wimomDcGtzBpTa7slxUIPUKY0t3nweGuS/GgQo= -github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= +github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2 h1:pbu+PFvYJ8q8GJ8O8ni6H2md7HXfhvnub9BPAiVVj5g= +github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= diff --git a/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go b/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go index 5d111b7e6f..2bcbd6e98c 100644 --- a/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go +++ b/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go @@ -454,7 +454,7 @@ func TestCharsetsAndCollations(t *testing.T) { require.NoError(t, rows.Close()) // Test that we get an error for unsupported charsets/collations - primaryDatabase.MustExec("CREATE TABLE t2 (pk int primary key, c1 varchar(255) COLLATE utf16_german2_ci);") + primaryDatabase.MustExec("CREATE TABLE t2 (pk int primary key, c1 varchar(255) COLLATE utf8mb4_bg_0900_as_cs);") waitForReplicaToCatchUp(t) replicaDatabase.MustExec("use db01;") rows, err = replicaDatabase.Queryx("SHOW TABLES WHERE Tables_in_db01 like 't2';") @@ -467,7 +467,7 @@ func TestCharsetsAndCollations(t *testing.T) { row = convertByteArraysToStrings(readNextRow(t, rows)) require.Equal(t, "1105", row["Last_SQL_Errno"]) require.NotEmpty(t, row["Last_SQL_Error_Timestamp"]) - require.Contains(t, row["Last_SQL_Error"], "The collation `utf16_german2_ci` has not yet been implemented") + require.Contains(t, row["Last_SQL_Error"], "The collation `utf8mb4_bg_0900_as_cs` has not yet been implemented") require.False(t, rows.Next()) require.NoError(t, rows.Close()) } From c1ed21669a4d129efe656debcc076f82fe5bb918 Mon Sep 17 00:00:00 2001 From: max-hoffman Date: Tue, 6 Jun 2023 05:24:11 +0000 Subject: [PATCH 61/64] [ga-bump-release] Update Dolt version to 1.2.5 and release v1.2.5 --- go/cmd/dolt/dolt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 024b393bad..1c7e44355b 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -61,7 +61,7 @@ import ( ) const ( - Version = "1.2.4" + Version = "1.2.5" ) var dumpDocsCommand = &commands.DumpDocsCmd{} From feb2704f626a16d5ca2b41b1b79f8eadbeefd102 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Tue, 6 Jun 2023 10:02:00 -0700 Subject: [PATCH 62/64] Bump gms --- go/go.mod | 2 +- go/go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go/go.mod b/go/go.mod index b133ef66ca..a8b1bceb0a 100644 --- a/go/go.mod +++ b/go/go.mod @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2 + github.com/dolthub/go-mysql-server v0.15.1-0.20230606163443-3036c6d7fff7 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index bcbe260f93..db05767469 100644 --- a/go/go.sum +++ b/go/go.sum @@ -170,6 +170,8 @@ github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2 h1:pbu+PFvYJ8q8GJ8O8ni6H2md7HXfhvnub9BPAiVVj5g= github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= +github.com/dolthub/go-mysql-server v0.15.1-0.20230606163443-3036c6d7fff7 h1:Tpl0xYr2MqfZhENtHr+H166D41nLRpL+XPchLhRHcwQ= +github.com/dolthub/go-mysql-server v0.15.1-0.20230606163443-3036c6d7fff7/go.mod h1:E1STXoY8HvbrOWwFOhHpMfIjgcjboBhFtSpRxPgiBOw= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= From 8ba2c5bba7ec237ffe4c586035ae0d9ea8884d67 Mon Sep 17 00:00:00 2001 From: coffeegoddd Date: Tue, 6 Jun 2023 10:45:58 -0700 Subject: [PATCH 63/64] /.github/workflows/ci-go-{race-enginetests,tests}.yaml: set vet off on go tests --- .github/workflows/ci-go-race-enginetests.yaml | 2 +- .github/workflows/ci-go-tests.yaml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci-go-race-enginetests.yaml b/.github/workflows/ci-go-race-enginetests.yaml index e1fbd51e93..0a4bc5f871 100644 --- a/.github/workflows/ci-go-race-enginetests.yaml +++ b/.github/workflows/ci-go-race-enginetests.yaml @@ -29,6 +29,6 @@ jobs: - name: Test All working-directory: ./go run: | - DOLT_SKIP_PREPARED_ENGINETESTS=1 go test -v -race -timeout 30m github.com/dolthub/dolt/go/libraries/doltcore/sqle/enginetest + DOLT_SKIP_PREPARED_ENGINETESTS=1 go test -vet=off -v -race -timeout 30m github.com/dolthub/dolt/go/libraries/doltcore/sqle/enginetest env: DOLT_DEFAULT_BIN_FORMAT: ${{ matrix.dolt_fmt }} diff --git a/.github/workflows/ci-go-tests.yaml b/.github/workflows/ci-go-tests.yaml index 4e5d9cb740..c253075edd 100644 --- a/.github/workflows/ci-go-tests.yaml +++ b/.github/workflows/ci-go-tests.yaml @@ -44,12 +44,12 @@ jobs: if [ "$MATRIX_OS" == 'ubuntu-22.04' ] then if [[ "${file_arr[$i]}" != *enginetest* ]]; then - go test -timeout 45m -race "${file_arr[$i]}" + go test -vet=off -timeout 45m -race "${file_arr[$i]}" else echo "skipping enginetests for -race" fi else - go test -timeout 45m "${file_arr[$i]}" + go test -vet=off -timeout 45m "${file_arr[$i]}" fi succeeded=$(echo "$?") if [ "$succeeded" -ne 0 ]; then @@ -79,8 +79,8 @@ jobs: - name: Test All working-directory: ./go run: | - go test -timeout 30m ./libraries/doltcore/sqle/altertests - go test -timeout 30m ./libraries/doltcore/sqle/integration_test + go test -vet=off -timeout 30m ./libraries/doltcore/sqle/altertests + go test -vet=off -timeout 30m ./libraries/doltcore/sqle/integration_test env: MATRIX_OS: ${{ matrix.os }} DOLT_TEST_RUN_NON_RACE_TESTS: "true" From d192005ee463c5527f6cfac5f7d03afd81a6d27e Mon Sep 17 00:00:00 2001 From: tbantle22 Date: Tue, 6 Jun 2023 19:21:35 +0000 Subject: [PATCH 64/64] [ga-bump-release] Update Dolt version to 1.3.0 and release v1.3.0 --- go/cmd/dolt/dolt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 1c7e44355b..6499b7f3d0 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -61,7 +61,7 @@ import ( ) const ( - Version = "1.2.5" + Version = "1.3.0" ) var dumpDocsCommand = &commands.DumpDocsCmd{}