From 790219c70bc88903f99e696f942b36ba5e83a5e3 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 22 May 2023 16:23:45 -0700 Subject: [PATCH 01/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] [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/28] 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/28] 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/28] `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/28] 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/28] 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/28] 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/28] 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 5260397aed2e516f71a9437cd9ffa43e95509694 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Thu, 1 Jun 2023 11:40:45 -0700 Subject: [PATCH 20/28] 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 bef50ffebd153fa20ae88fb776a96e7d2c395f59 Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Thu, 1 Jun 2023 19:24:04 -0700 Subject: [PATCH 21/28] 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 d5e3d075beaa27f86c8b3c9315530d2675c2714e Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 12:21:53 -0700 Subject: [PATCH 22/28] 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 98f717e2363dae985dd70265214a244622cbc49e Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Fri, 2 Jun 2023 13:52:07 -0700 Subject: [PATCH 23/28] 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 24/28] 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 25/28] 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 26/28] 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 be8056793e465d9379ac973b04341106c8ccfe18 Mon Sep 17 00:00:00 2001 From: nicktobey Date: Mon, 5 Jun 2023 18:22:38 +0000 Subject: [PATCH 27/28] [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 0d88a6aab8c360560a98d0343621752cbd44a05f Mon Sep 17 00:00:00 2001 From: Nick Tobey Date: Mon, 5 Jun 2023 12:28:14 -0700 Subject: [PATCH 28/28] 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 ]