From c2f4ab9b17f31ca4f52519a56d34adc2ee8974f3 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 13:56:49 -0700 Subject: [PATCH 01/20] Fix BATS test name. --- integration-tests/bats/cherry-pick.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/cherry-pick.bats b/integration-tests/bats/cherry-pick.bats index 18753ddfe5d..ed59a76816e 100644 --- a/integration-tests/bats/cherry-pick.bats +++ b/integration-tests/bats/cherry-pick.bats @@ -156,7 +156,7 @@ teardown() { [[ ! "$output" =~ "branch1table" ]] || false } -@test "cherry-pick: error when using `--abort` with no in-progress cherry-pick" { +@test "cherry-pick: error when using --abort with no in-progress cherry-pick" { run dolt cherry-pick --abort [ $status -eq 1 ] [[ $output =~ "error: There is no cherry-pick merge to abort" ]] || false From 9f28e2ab0aab8aeffa75f44994df7ccf4481dff3 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 13:57:43 -0700 Subject: [PATCH 02/20] Update getTinyIntColAsBool and getInt64ColAsInt64 to handle `int` types. --- go/cmd/dolt/commands/status.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go/cmd/dolt/commands/status.go b/go/cmd/dolt/commands/status.go index c7499f18862..568e071a8d7 100644 --- a/go/cmd/dolt/commands/status.go +++ b/go/cmd/dolt/commands/status.go @@ -665,6 +665,8 @@ func getTinyIntColAsBool(col interface{}) (bool, error) { return v == 1, nil case string: return v == "1", nil + case int8: + return v == 1, nil default: return false, fmt.Errorf("unexpected type %T, was expecting bool, int, or string", v) } @@ -693,6 +695,8 @@ func getJsonDocumentColAsString(sqlCtx *sql.Context, col interface{}) (string, e // or as a string (when using ConnectionQueryist). func getInt64ColAsInt64(col interface{}) (int64, error) { switch v := col.(type) { + case int: + return int64(v), nil case uint64: return int64(v), nil case int64: From 331c3e453ba9e88dbb5b95770ea235b34bfd3235 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 14:27:35 -0700 Subject: [PATCH 03/20] Starting to migrate cherry-pick to use SQL. --- go/cmd/dolt/commands/cherry-pick.go | 211 +++++----------------------- 1 file changed, 39 insertions(+), 172 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 1a9b02cd59f..1cf3723a10e 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -16,17 +16,18 @@ package commands import ( "context" - + "fmt" "gopkg.in/src-d/go-errors.v1" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" 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/branch_control" "github.com/dolthub/dolt/go/libraries/doltcore/env" - "github.com/dolthub/dolt/go/libraries/doltcore/merge" - "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" "github.com/dolthub/dolt/go/libraries/utils/argparser" + "github.com/dolthub/go-mysql-server/sql" + "github.com/gocraft/dbr/v2" + "github.com/gocraft/dbr/v2/dialect" ) var cherryPickDocs = cli.CommandDocumentationContent{ @@ -80,25 +81,25 @@ func (cmd CherryPickCmd) Exec(ctx context.Context, commandStr string, args []str ap := cli.CreateCherryPickArgParser() help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, cherryPickDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) - if dEnv.IsLocked() { - return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help) + + queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) + if err != nil { + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } + if closeFunc != nil { + defer closeFunc() + } + // This command creates a commit, so we need user identity - if !cli.CheckUserNameAndEmail(cliCtx.Config()) { + err = branch_control.CheckAccess(sqlCtx, branch_control.Permissions_Write) + if err != nil { + cli.Println(err.Error()) return 1 } if apr.Contains(cli.AbortParam) { - ws, err := dEnv.WorkingSet(ctx) - if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("fatal: unable to load working set: %v", err).Build(), nil) - } - - if !ws.MergeActive() { - return HandleVErrAndExitCode(errhand.BuildDError("error: There is no cherry-pick merge to abort").Build(), nil) - } - - return HandleVErrAndExitCode(abortMerge(ctx, dEnv), usage) + err = cherryPickAbort(queryist, sqlCtx) + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } // TODO : support single commit cherry-pick only for now @@ -109,173 +110,39 @@ func (cmd CherryPickCmd) Exec(ctx context.Context, commandStr string, args []str return HandleVErrAndExitCode(errhand.BuildDError("cherry-picking multiple commits is not supported yet").SetPrintUsage().Build(), usage) } - cherryStr := apr.Arg(0) - if len(cherryStr) == 0 { - verr := errhand.BuildDError("error: cannot cherry-pick empty string").Build() - return HandleVErrAndExitCode(verr, usage) - } - - verr := cherryPick(ctx, dEnv, cliCtx, cherryStr) - return HandleVErrAndExitCode(verr, usage) + err = cherryPick(queryist, sqlCtx, apr) + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), 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, cliCtx cli.CliContext, cherryStr string) errhand.VerboseError { - // check for clean working state - headRoot, err := dEnv.HeadRoot(ctx) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - workingRoot, err := dEnv.WorkingRoot(ctx) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - stagedRoot, err := dEnv.StagedRoot(ctx) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - headHash, err := headRoot.HashOf() - if err != nil { - return errhand.VerboseErrorFromError(err) - } - workingHash, err := workingRoot.HashOf() - if err != nil { - return errhand.VerboseErrorFromError(err) - } - stagedHash, err := stagedRoot.HashOf() - if err != nil { - return errhand.VerboseErrorFromError(err) - } - - if !headHash.Equal(stagedHash) { - return errhand.BuildDError("Please commit your staged changes before using cherry-pick.").Build() - } - - if !headHash.Equal(workingHash) { - return errhand.BuildDError("error: your local changes would be overwritten by cherry-pick.\nhint: commit your changes (dolt commit -am \"\") or reset them (dolt reset --hard) to proceed.").Build() +func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgParseResults) error { + cherryStr := apr.Arg(0) + if len(cherryStr) == 0 { + return fmt.Errorf("error: cannot cherry-pick empty string") } - mergeResult, commitMsg, err := mergeCherryPickedCommit(ctx, dEnv, workingRoot, cherryStr) + q, err := dbr.InterpolateForDialect("call dolt_cherry_pick(?)", []interface{}{cherryStr}, dialect.MySQL) if err != nil { - return errhand.VerboseErrorFromError(err) + return fmt.Errorf("error: failed to interpolate query: %w", err) } - newWorkingRoot := mergeResult.Root - mapOfMergeStats := mergeResult.Stats - - workingHash, err = newWorkingRoot.HashOf() + _, err = getRowsForSql(queryist, sqlCtx, q) if err != nil { - return errhand.VerboseErrorFromError(err) - } - - if headHash.Equal(workingHash) { - cli.Println("No changes were made.") - return nil + return err } - - err = dEnv.UpdateWorkingRoot(ctx, newWorkingRoot) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - - // Stage all tables that don't have merge conflicts or constraint violations - for tableName, mergeStats := range mapOfMergeStats { - if !mergeStats.HasArtifacts() { - res := AddCmd{}.Exec(ctx, "add", []string{tableName}, dEnv, cliCtx) - if res != 0 { - return errhand.BuildDError("dolt add failed").AddCause(err).Build() - } - } - } - - printSuccessStats(mapOfMergeStats) - - if mergeResult.HasMergeArtifacts() { - return errhand.VerboseErrorFromError(ErrCherryPickConflictsOrViolations.New()) - } else { - commitParams := []string{"-m", commitMsg} - res := CommitCmd{}.Exec(ctx, "commit", commitParams, dEnv, cliCtx) - if res != 0 { - return errhand.BuildDError("dolt commit failed").AddCause(err).Build() - } - } - return nil } -// mergeCherryPickedCommit executes a merge to cherry-pick the specified ref specification from -// |cherryStr| and apply it to the specified |workingRoot| in this |dEnv|. The MergeResult is -// returned, along with the commit message for the specified |cherryStr|, and any error encountered. -func mergeCherryPickedCommit(ctx context.Context, dEnv *env.DoltEnv, workingRoot *doltdb.RootValue, cherryStr string) (*merge.Result, string, error) { - tmpDir, err := dEnv.TempTableFilesDir() - if err != nil { - return nil, "", err - } - opts := editor.Options{Deaf: dEnv.BulkDbEaFactory(), Tempdir: tmpDir} - - cherrySpec, err := doltdb.NewCommitSpec(cherryStr) +func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { + q := "call dolt_merge('--abort')" + _, err := getRowsForSql(queryist, sqlCtx, q) if err != nil { - return nil, "", err - } - headRef, err := dEnv.RepoStateReader().CWBHeadRef() - if err != nil { - return nil, "", err - } - cherryCm, err := dEnv.DoltDB.Resolve(ctx, cherrySpec, headRef) - if err != nil { - return nil, "", err - } - - if len(cherryCm.DatasParents()) > 1 { - return nil, "", errhand.BuildDError("cherry-picking a merge commit is not supported.").Build() - } - if len(cherryCm.DatasParents()) == 0 { - return nil, "", errhand.BuildDError("cherry-picking a commit without parents is not supported.").Build() - } - - cherryCM, err := cherryCm.GetCommitMeta(ctx) - if err != nil { - return nil, "", err - } - commitMsg := cherryCM.Description - - cherryRoot, err := cherryCm.GetRootValue(ctx) - if err != nil { - return nil, "", err - } - - parentCm, err := dEnv.DoltDB.ResolveParent(ctx, cherryCm, 0) - if err != nil { - return nil, "", err - } - parentRoot, err := parentCm.GetRootValue(ctx) - if err != nil { - return nil, "", err - } - - // use parent of cherry-pick as ancestor to merge - mo := merge.MergeOpts{IsCherryPick: true} - result, err := merge.MergeRoots(ctx, workingRoot, cherryRoot, parentRoot, cherryCm, parentCm, opts, mo) - if err != nil { - return nil, "", err - } - - // If any of the merge stats show a data or schema conflict or a constraint - // violation, record that a merge is in progress. - for _, stats := range result.Stats { - if stats.HasArtifacts() { - ws, err := dEnv.WorkingSet(ctx) - if err != nil { - return nil, "", err - } - newWorkingSet := ws.StartMerge(cherryCm, cherryStr) - err = dEnv.UpdateWorkingSet(ctx, newWorkingSet) - if err != nil { - return nil, "", err - } - - break + errorText := err.Error() + switch errorText { + case "fatal: There is no merge to abort": + return fmt.Errorf("error: There is no cherry-pick merge to abort") + default: + return err } } - - return result, commitMsg, nil + return nil } + From cfe133ac8103d16b6746d478bfbb8d3681ed3b15 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 14:30:05 -0700 Subject: [PATCH 04/20] Check if there are staged or working changes, error as appropriate. --- go/cmd/dolt/commands/cherry-pick.go | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 1cf3723a10e..9eb79d18b5f 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -120,6 +120,22 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa return fmt.Errorf("error: cannot cherry-pick empty string") } + hasStagedChanges, hasUnstagedChanges, err := getDoltStatus(queryist, sqlCtx) + if err != nil { + return fmt.Errorf("error: failed to get dolt status: %w", err) + } + if hasStagedChanges { + return fmt.Errorf("Please commit your staged changes before using cherry-pick.") + } + if hasUnstagedChanges { + return fmt.Errorf("error: your local changes would be overwritten by cherry-pick.\nhint: commit your changes (dolt commit -am \"\") or reset them (dolt reset --hard) to proceed.") + } + + _, err = getRowsForSql(queryist, sqlCtx, "set @@dolt_allow_commit_conflicts = 1") + if err != nil { + return fmt.Errorf("error: failed to set @@dolt_allow_commit_conflicts: %w", err) + } + q, err := dbr.InterpolateForDialect("call dolt_cherry_pick(?)", []interface{}{cherryStr}, dialect.MySQL) if err != nil { return fmt.Errorf("error: failed to interpolate query: %w", err) @@ -146,3 +162,33 @@ func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { return nil } +func getDoltStatus(queryist cli.Queryist, sqlCtx *sql.Context) (hasStagedChanges bool, hasUnstagedChanges bool, err error) { + hasStagedChanges = false + hasUnstagedChanges = false + err = nil + + var statusRows []sql.Row + statusRows, err = getRowsForSql(queryist, sqlCtx, "select * from dolt_status;") + if err != nil { + return + } + if len(statusRows) == 0 { + return + } + + for _, row := range statusRows { + staged := row[1] + var isStaged bool + isStaged, err = getTinyIntColAsBool(staged) + if err != nil { + return + } + if isStaged { + hasStagedChanges = true + } else { + hasUnstagedChanges = true + } + } + + return +} From aa71d3e3db0bb052ec96f113c2cba6cd26993d49 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 14:33:25 -0700 Subject: [PATCH 05/20] Check `dolt_cherry_pick` error response and return appropriate errors. --- go/cmd/dolt/commands/cherry-pick.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 9eb79d18b5f..8d884c07618 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "gopkg.in/src-d/go-errors.v1" + "strings" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" @@ -142,7 +143,20 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa } _, err = getRowsForSql(queryist, sqlCtx, q) if err != nil { - return err + errorText := err.Error() + switch { + case "no changes were made, nothing to commit" == errorText: + cli.Println("No changes were made.") + return nil + case + strings.HasPrefix(errorText, "Merge conflict detected, transaction rolled back."), + strings.HasPrefix(errorText, "Committing this transaction resulted in a working set with constraint violations"): + return ErrCherryPickConflictsOrViolations.New() + case "cherry-picking a merge commit is not supported" == errorText: + return fmt.Errorf("cherry-picking a merge commit is not supported.") + default: + return err + } } return nil } From 8e8305be840ca12cae9aa880b7808979578ce640 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 14:34:28 -0700 Subject: [PATCH 06/20] Check `dolt_cherry_pick` result data and throw appropriate errors if there are conflicts. --- go/cmd/dolt/commands/cherry-pick.go | 51 +++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 8d884c07618..a5c319debf5 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -17,6 +17,7 @@ package commands import ( "context" "fmt" + "github.com/dolthub/dolt/go/store/util/outputpager" "gopkg.in/src-d/go-errors.v1" "strings" @@ -141,7 +142,7 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa if err != nil { return fmt.Errorf("error: failed to interpolate query: %w", err) } - _, err = getRowsForSql(queryist, sqlCtx, q) + rows, err := getRowsForSql(queryist, sqlCtx, q) if err != nil { errorText := err.Error() switch { @@ -158,7 +159,53 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa return err } } - return nil + + if len(rows) != 1 { + return fmt.Errorf("error: unexpected number of rows returned from dolt_cherry_pick: %d", len(rows)) + } + + succeeded := false + commitHash := "" + for _, row := range rows { + commitHash = row[0].(string) + dataConflicts, err := getInt64ColAsInt64(row[1]) + if err != nil { + return fmt.Errorf("Unable to parse data_conflicts column: %w", err) + } + schemaConflicts, err := getInt64ColAsInt64(row[2]) + if err != nil { + return fmt.Errorf("Unable to parse schema_conflicts column: %w", err) + } + constraintViolations, err := getInt64ColAsInt64(row[3]) + if err != nil { + return fmt.Errorf("Unable to parse constraint_violations column: %w", err) + } + + // if we have a hash and all 0s, then the cherry-pick succeeded + if len(commitHash) > 0 && dataConflicts == 0 && schemaConflicts == 0 && constraintViolations == 0 { + succeeded = true + } + } + + if succeeded { + // on success, print the commit info + commit, err := getCommitInfo(queryist, sqlCtx, commitHash) + if err != nil { + return fmt.Errorf("error: failed to get commit metadata for ref '%s': %v", commitHash, err) + } + + cli.ExecuteWithStdioRestored(func() { + pager := outputpager.Start() + defer pager.Stop() + + printCommitInfo(pager, 0, false, "auto", commit) + }) + + return nil + } else { + // this failure could only have been caused by constraint violations or conflicts during cherry-pick + return ErrCherryPickConflictsOrViolations.New() + } } func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { From a24617ecce8291af5445f44799768dab44593135 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 15:15:43 -0700 Subject: [PATCH 07/20] Set @@dolt_force_transaction_commit=1, so even if dolt_cherry_pick results in a conflict merge, the transaction changes are not rolled back. --- go/cmd/dolt/commands/cherry-pick.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index a5c319debf5..2facbb54093 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -138,6 +138,11 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa return fmt.Errorf("error: failed to set @@dolt_allow_commit_conflicts: %w", err) } + _, err = getRowsForSql(queryist, sqlCtx, "set @@dolt_force_transaction_commit = 1") + if err != nil { + return fmt.Errorf("error: failed to set @@dolt_force_transaction_commit: %w", err) + } + q, err := dbr.InterpolateForDialect("call dolt_cherry_pick(?)", []interface{}{cherryStr}, dialect.MySQL) if err != nil { return fmt.Errorf("error: failed to interpolate query: %w", err) From a6d073801e115dee60b9d6329213aab7309e730c Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 28 Jun 2023 15:16:40 -0700 Subject: [PATCH 08/20] Sort imports. --- go/cmd/dolt/commands/cherry-pick.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 2facbb54093..866ff6ea847 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -17,7 +17,6 @@ package commands import ( "context" "fmt" - "github.com/dolthub/dolt/go/store/util/outputpager" "gopkg.in/src-d/go-errors.v1" "strings" @@ -27,6 +26,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/branch_control" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/utils/argparser" + "github.com/dolthub/dolt/go/store/util/outputpager" "github.com/dolthub/go-mysql-server/sql" "github.com/gocraft/dbr/v2" "github.com/gocraft/dbr/v2/dialect" From 35573e85beb225af04a75f65e4a386c65ef99b9c Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Thu, 29 Jun 2023 11:17:06 -0700 Subject: [PATCH 09/20] Add cherry-pick tests under sql-local-remote.bats --- integration-tests/bats/sql-local-remote.bats | 72 +++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 1cd8fafca6a..82f2127f2aa 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -570,7 +570,7 @@ EOF [ "$status" -eq 1 ] [[ "$output" =~ "When a password is provided, a user must also be provided" ]] || false - stop_sql_server 1 + stop_sql_server 1 run dolt --password "anything" sql -q "show tables" [ "$status" -eq 1 ] @@ -581,3 +581,73 @@ EOF [ "$status" -eq 1 ] [[ "$output" =~ "When a password is provided, a user must also be provided" ]] || false } + +@test "sql-local-remote: verify dolt cherry-pick behavior" { + cd altDB + + # setup for cherry-pick.bats + dolt clean + dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, v varchar(10), index(v))" + dolt add . + dolt commit -am "Created table" + dolt checkout -b branch1 + dolt sql -q "INSERT INTO test VALUES (1, 'a')" + dolt commit -am "Inserted 1" + dolt sql -q "INSERT INTO test VALUES (2, 'b')" + dolt commit -am "Inserted 2" + dolt sql -q "INSERT INTO test VALUES (3, 'c')" + dolt commit -am "Inserted 3" + run dolt sql -q "SELECT * FROM test" -r csv + [[ "$output" =~ "1,a" ]] || false + [[ "$output" =~ "2,b" ]] || false + [[ "$output" =~ "3,c" ]] || false + + # setup for "cherry-pick: schema change, with data conflict" test + dolt checkout main + dolt sql -q "CREATE TABLE other (pk int primary key, c1 int, c2 int)" + dolt sql -q "INSERT INTO other VALUES (1, 2, 3)" + dolt commit -Am "add other table (on main)" + # Create two commits on branch2: one to assert does NOT get included, and one to cherry pick + dolt checkout -b branch2 + dolt sql -q "INSERT INTO other VALUES (100, 200, 300);" + dolt commit -am "add row 100 to other (on branch2)" + # This ALTER TABLE statement modifies other rows that aren't included in the cherry-picked + # commit – row (100, 200, 300) is modified to (100, 300). This shows up as a conflict + # in the cherry-pick (modified row on one side, row doesn't exist on the other side). + dolt sql -q "ALTER TABLE other DROP COLUMN c1;" + dolt sql -q "INSERT INTO other VALUES (10, 30);" + dolt sql -q "INSERT INTO test VALUES (100, 'q');" + dolt commit -am "alter table, add row 10 to other, add row 100 to test (on branch2)" + + # actual cherry-pick test + dolt checkout main + run dolt cherry-pick branch2 + [ $status -eq 1 ] + [[ $output =~ "Unable to apply commit cleanly due to conflicts or constraint violations" ]] || false + localCherryPickOutput=$output + + # Assert that table 'test' is staged, but table 'other' is not staged, since it had conflicts + run dolt sql -q "SELECT table_name, case when staged = 0 then 'staged' else 'working' end as location, status from dolt_status;" + [ $status -eq 0 ] + [[ $output =~ "| test | working | modified |" ]] || false + [[ $output =~ "| other | staged | modified |" ]] || false + + # setup for remote test + dolt checkout main + dolt reset --hard main + + # start server + start_sql_server altDB + + run dolt cherry-pick branch2 + [ $status -eq 1 ] + [[ $output =~ "Unable to apply commit cleanly due to conflicts or constraint violations" ]] || false + remoteCherryPickOutput=$output + # Assert that table 'test' is staged, but table 'other' is not staged, since it had conflicts + run dolt sql -q "SELECT table_name, case when staged = 0 then 'staged' else 'working' end as location, status from dolt_status;" + [ $status -eq 0 ] + [[ $output =~ "| test | working | modified |" ]] || false + [[ $output =~ "| other | staged | modified |" ]] || false + + [[ "$localCherryPickOutput" == "$remoteCherryPickOutput" ]] || false +} From ec00e256014822c8417536a777822b5dd7d3eb85 Mon Sep 17 00:00:00 2001 From: PavelSafronov Date: Thu, 29 Jun 2023 18:36:40 +0000 Subject: [PATCH 10/20] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/cherry-pick.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 866ff6ea847..948c90a10a5 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -17,9 +17,13 @@ package commands import ( "context" "fmt" - "gopkg.in/src-d/go-errors.v1" "strings" + "github.com/dolthub/go-mysql-server/sql" + "github.com/gocraft/dbr/v2" + "github.com/gocraft/dbr/v2/dialect" + "gopkg.in/src-d/go-errors.v1" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" @@ -27,9 +31,6 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/store/util/outputpager" - "github.com/dolthub/go-mysql-server/sql" - "github.com/gocraft/dbr/v2" - "github.com/gocraft/dbr/v2/dialect" ) var cherryPickDocs = cli.CommandDocumentationContent{ From 1c08df36f658b66af7fdf6c4f4c64c83737b20d1 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Thu, 29 Jun 2023 15:25:30 -0700 Subject: [PATCH 11/20] Update getRowsForSql to GetRowsForSql --- go/cmd/dolt/commands/cherry-pick.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 5553ade3217..839542f3354 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -134,12 +134,12 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa return fmt.Errorf("error: your local changes would be overwritten by cherry-pick.\nhint: commit your changes (dolt commit -am \"\") or reset them (dolt reset --hard) to proceed.") } - _, err = getRowsForSql(queryist, sqlCtx, "set @@dolt_allow_commit_conflicts = 1") + _, err = GetRowsForSql(queryist, sqlCtx, "set @@dolt_allow_commit_conflicts = 1") if err != nil { return fmt.Errorf("error: failed to set @@dolt_allow_commit_conflicts: %w", err) } - _, err = getRowsForSql(queryist, sqlCtx, "set @@dolt_force_transaction_commit = 1") + _, err = GetRowsForSql(queryist, sqlCtx, "set @@dolt_force_transaction_commit = 1") if err != nil { return fmt.Errorf("error: failed to set @@dolt_force_transaction_commit: %w", err) } @@ -148,7 +148,7 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa if err != nil { return fmt.Errorf("error: failed to interpolate query: %w", err) } - rows, err := getRowsForSql(queryist, sqlCtx, q) + rows, err := GetRowsForSql(queryist, sqlCtx, q) if err != nil { errorText := err.Error() switch { @@ -216,7 +216,7 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { q := "call dolt_merge('--abort')" - _, err := getRowsForSql(queryist, sqlCtx, q) + _, err := GetRowsForSql(queryist, sqlCtx, q) if err != nil { errorText := err.Error() switch errorText { @@ -235,7 +235,7 @@ func getDoltStatus(queryist cli.Queryist, sqlCtx *sql.Context) (hasStagedChanges err = nil var statusRows []sql.Row - statusRows, err = getRowsForSql(queryist, sqlCtx, "select * from dolt_status;") + statusRows, err = GetRowsForSql(queryist, sqlCtx, "select * from dolt_status;") if err != nil { return } From 1575a45e55b5af762233c418d970f10a42a553b2 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 5 Jul 2023 11:01:26 -0700 Subject: [PATCH 12/20] PR feedback: move getInt64ColAsInt64 to utils. --- go/cmd/dolt/commands/status.go | 23 ----------------------- go/cmd/dolt/commands/utils.go | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/go/cmd/dolt/commands/status.go b/go/cmd/dolt/commands/status.go index feced44dd91..e2b230304fd 100644 --- a/go/cmd/dolt/commands/status.go +++ b/go/cmd/dolt/commands/status.go @@ -17,7 +17,6 @@ package commands import ( "context" "fmt" - "strconv" "strings" "github.com/dolthub/go-mysql-server/sql" @@ -671,25 +670,3 @@ func getJsonDocumentColAsString(sqlCtx *sql.Context, col interface{}) (string, e return "", fmt.Errorf("unexpected type %T, was expecting JSONDocument or string", v) } } - -// getInt64ColAsInt64 returns the value of an int64 column as a string -// This is necessary because Queryist may return an int64 column as an int64 (when using SQLEngine) -// or as a string (when using ConnectionQueryist). -func getInt64ColAsInt64(col interface{}) (int64, error) { - switch v := col.(type) { - case int: - return int64(v), nil - case uint64: - return int64(v), nil - case int64: - return v, nil - case string: - iv, err := strconv.ParseInt(v, 10, 64) - if err != nil { - return 0, err - } - return iv, nil - default: - return 0, fmt.Errorf("unexpected type %T, was expecting int64, uint64 or string", v) - } -} diff --git a/go/cmd/dolt/commands/utils.go b/go/cmd/dolt/commands/utils.go index bcbee9469d7..29c69252dea 100644 --- a/go/cmd/dolt/commands/utils.go +++ b/go/cmd/dolt/commands/utils.go @@ -20,6 +20,7 @@ import ( "fmt" "net" "path/filepath" + "strconv" "time" "github.com/dolthub/go-mysql-server/sql" @@ -309,6 +310,28 @@ func GetTinyIntColAsBool(col interface{}) (bool, error) { } } +// getInt64ColAsInt64 returns the value of an int64 column as a string +// This is necessary because Queryist may return an int64 column as an int64 (when using SQLEngine) +// or as a string (when using ConnectionQueryist). +func getInt64ColAsInt64(col interface{}) (int64, error) { + switch v := col.(type) { + case int: + return int64(v), nil + case uint64: + return int64(v), nil + case int64: + return v, nil + case string: + iv, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return 0, err + } + return iv, nil + default: + return 0, fmt.Errorf("unexpected type %T, was expecting int64, uint64 or string", v) + } +} + func getActiveBranchName(sqlCtx *sql.Context, queryEngine cli.Queryist) (string, error) { query := "SELECT active_branch()" rows, err := GetRowsForSql(queryEngine, sqlCtx, query) From 19afcb4b5c92f3b5aacdb1dfc26d755a1f361d30 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 5 Jul 2023 11:02:11 -0700 Subject: [PATCH 13/20] PR feedback: use backtick string literal for error message that has newline. --- go/cmd/dolt/commands/cherry-pick.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index f7ec1bb4ef0..200d9122638 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -131,7 +131,8 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa return fmt.Errorf("Please commit your staged changes before using cherry-pick.") } if hasUnstagedChanges { - return fmt.Errorf("error: your local changes would be overwritten by cherry-pick.\nhint: commit your changes (dolt commit -am \"\") or reset them (dolt reset --hard) to proceed.") + return fmt.Errorf(`error: your local changes would be overwritten by cherry-pick. +hint: commit your changes (dolt commit -am \"\") or reset them (dolt reset --hard) to proceed.`) } _, err = GetRowsForSql(queryist, sqlCtx, "set @@dolt_allow_commit_conflicts = 1") From a714dd3ce40770d3eefb71a8ca4508a3142231fc Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 5 Jul 2023 11:02:36 -0700 Subject: [PATCH 14/20] PR feedback: use more descriptive variable name for query. --- go/cmd/dolt/commands/cherry-pick.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 200d9122638..f2fb33cff01 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -216,8 +216,8 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re } func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { - q := "call dolt_merge('--abort')" - _, err := GetRowsForSql(queryist, sqlCtx, q) + query := "call dolt_merge('--abort')" + _, err := GetRowsForSql(queryist, sqlCtx, query) if err != nil { errorText := err.Error() switch errorText { From 5a8a6e1e1091ee40152433382d3f41320b0c0c68 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Wed, 5 Jul 2023 11:43:19 -0700 Subject: [PATCH 15/20] Remove unnecessary error text checks. --- go/cmd/dolt/commands/cherry-pick.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index f2fb33cff01..31f14d4cb88 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -17,8 +17,6 @@ package commands import ( "context" "fmt" - "strings" - "github.com/dolthub/go-mysql-server/sql" "github.com/gocraft/dbr/v2" "github.com/gocraft/dbr/v2/dialect" @@ -156,10 +154,6 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re case "no changes were made, nothing to commit" == errorText: cli.Println("No changes were made.") return nil - case - strings.HasPrefix(errorText, "Merge conflict detected, transaction rolled back."), - strings.HasPrefix(errorText, "Committing this transaction resulted in a working set with constraint violations"): - return ErrCherryPickConflictsOrViolations.New() case "cherry-picking a merge commit is not supported" == errorText: return fmt.Errorf("cherry-picking a merge commit is not supported.") default: From f3aa63c21c4d08ed5c98ee120fbdf2dec0e088f8 Mon Sep 17 00:00:00 2001 From: PavelSafronov Date: Wed, 5 Jul 2023 18:52:36 +0000 Subject: [PATCH 16/20] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/cherry-pick.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 31f14d4cb88..33fbd9c078f 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -17,6 +17,7 @@ package commands import ( "context" "fmt" + "github.com/dolthub/go-mysql-server/sql" "github.com/gocraft/dbr/v2" "github.com/gocraft/dbr/v2/dialect" From 52efcee750ad35d423468775c380ce67336ca8d4 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Fri, 7 Jul 2023 09:58:49 -0700 Subject: [PATCH 17/20] Add ignored tables to cherry-pick BATS tests. --- integration-tests/bats/cherry-pick.bats | 19 ++++-- integration-tests/bats/sql-cherry-pick.bats | 68 ++++++++++++++++++++- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/integration-tests/bats/cherry-pick.bats b/integration-tests/bats/cherry-pick.bats index ed59a76816e..9703afd5ef6 100644 --- a/integration-tests/bats/cherry-pick.bats +++ b/integration-tests/bats/cherry-pick.bats @@ -4,7 +4,10 @@ load $BATS_TEST_DIRNAME/helper/common.bash setup() { setup_common - dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, v varchar(10), index(v))" + dolt sql <>> $output" + [ $status -eq 0 ] + [[ ! $output =~ "Changes to be committed" ]] || false + [[ $output =~ "Unmerged paths" ]] || false + [[ $output =~ "both modified: other" ]] || false + [[ $output =~ "both modified: test" ]] || false + + dolt conflicts resolve --theirs . + + run dolt sql -q "select * from other" + [ $status -eq 0 ] + [[ $output =~ "1 | 2" ]] || false + + run dolt sql -q "select * from test" + [ $status -eq 0 ] + [[ $output =~ "4 | f" ]] || false + + dolt commit -m "committing cherry-picked change" +} + + @test "sql-cherry-pick: commit with CREATE TABLE" { dolt sql -q "CREATE TABLE table_a (pk BIGINT PRIMARY KEY, v varchar(10))" dolt add . From 59377b6311e10f9724f104a34ce44ae2a40d8da2 Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Fri, 7 Jul 2023 09:59:38 -0700 Subject: [PATCH 18/20] Change how cherry-pick figures out if there are changes. --- go/cmd/dolt/commands/cherry-pick.go | 46 ++++++++------------------ go/cmd/dolt/commands/utils.go | 51 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 33 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 33fbd9c078f..d35cd0a27b5 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -22,6 +22,7 @@ import ( "github.com/gocraft/dbr/v2" "github.com/gocraft/dbr/v2/dialect" "gopkg.in/src-d/go-errors.v1" + "strings" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" @@ -122,9 +123,9 @@ func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgPa return fmt.Errorf("error: cannot cherry-pick empty string") } - hasStagedChanges, hasUnstagedChanges, err := getDoltStatus(queryist, sqlCtx) + hasStagedChanges, hasUnstagedChanges, err := hasStagedAndUnstagedChanged(queryist, sqlCtx) if err != nil { - return fmt.Errorf("error: failed to get dolt status: %w", err) + return fmt.Errorf("error: failed to check for staged and unstaged changes: %w", err) } if hasStagedChanges { return fmt.Errorf("Please commit your staged changes before using cherry-pick.") @@ -152,11 +153,9 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re if err != nil { errorText := err.Error() switch { - case "no changes were made, nothing to commit" == errorText: + case strings.Contains("nothing to commit", errorText): cli.Println("No changes were made.") return nil - case "cherry-picking a merge commit is not supported" == errorText: - return fmt.Errorf("cherry-picking a merge commit is not supported.") default: return err } @@ -211,8 +210,8 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re } func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { - query := "call dolt_merge('--abort')" - _, err := GetRowsForSql(queryist, sqlCtx, query) + query := "call dolt_cherry_pick('--abort')" + rows, err := GetRowsForSql(queryist, sqlCtx, query) if err != nil { errorText := err.Error() switch errorText { @@ -222,36 +221,17 @@ func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { return err } } + cli.Printf("rows: %s\n", rows) return nil } -func getDoltStatus(queryist cli.Queryist, sqlCtx *sql.Context) (hasStagedChanges bool, hasUnstagedChanges bool, err error) { - hasStagedChanges = false - hasUnstagedChanges = false - err = nil - - var statusRows []sql.Row - statusRows, err = GetRowsForSql(queryist, sqlCtx, "select * from dolt_status;") +func hasStagedAndUnstagedChanged(queryist cli.Queryist, sqlCtx *sql.Context) (hasStagedChanges bool, hasUnstagedChanges bool, err error) { + stagedTables, unstagedTables, err := GetDoltStatus(queryist, sqlCtx) if err != nil { - return - } - if len(statusRows) == 0 { - return - } - - for _, row := range statusRows { - staged := row[1] - var isStaged bool - isStaged, err = GetTinyIntColAsBool(staged) - if err != nil { - return - } - if isStaged { - hasStagedChanges = true - } else { - hasUnstagedChanges = true - } + return false, false, fmt.Errorf("error: failed to get dolt status: %w", err) } - return + hasStagedChanges = len(stagedTables) > 0 + hasUnstagedChanges = len(unstagedTables) > 0 + return hasStagedChanges, hasUnstagedChanges, nil } diff --git a/go/cmd/dolt/commands/utils.go b/go/cmd/dolt/commands/utils.go index 29c69252dea..8e2a27a699f 100644 --- a/go/cmd/dolt/commands/utils.go +++ b/go/cmd/dolt/commands/utils.go @@ -422,3 +422,54 @@ func buildAuthResponse(salt []byte, password string) []byte { return shaPwd } + +// GetDoltStatus retrieves the status of the current working set of changes in the working set, and returns two +// lists of modified tables: staged and unstaged. If both lists are empty, there are no changes in the working set. +// The list of unstaged tables does not include tables that are ignored, as configured by the dolt_ignore table. +func GetDoltStatus(queryist cli.Queryist, sqlCtx *sql.Context) (stagedChangedTables map[string]bool, unstagedChangedTables map[string]bool, err error) { + stagedChangedTables = make(map[string]bool) + unstagedChangedTables = make(map[string]bool) + err = nil + + ignoredPatterns, err := getIgnoredTablePatternsFromSql(queryist, sqlCtx) + if err != nil { + return stagedChangedTables, unstagedChangedTables, fmt.Errorf("error: failed to get ignored table patterns: %w", err) + } + + var statusRows []sql.Row + statusRows, err = GetRowsForSql(queryist, sqlCtx, "select * from dolt_status;") + if err != nil { + return stagedChangedTables, unstagedChangedTables, fmt.Errorf("error: failed to get dolt status: %w", err) + } + + for _, row := range statusRows { + tableName := row[0].(string) + staged := row[1] + var isStaged bool + isStaged, err = GetTinyIntColAsBool(staged) + if err != nil { + return + } + if isStaged { + stagedChangedTables[tableName] = true + } else { + // filter out ignored tables from untracked tables + ignored, err := ignoredPatterns.IsTableNameIgnored(tableName) + if conflict := doltdb.AsDoltIgnoreInConflict(err); conflict != nil { + continue + } else if err != nil { + return stagedChangedTables, unstagedChangedTables, fmt.Errorf("error: failed to check if table '%s' is ignored: %w", tableName, err) + } else if ignored == doltdb.DontIgnore { + // no-op + } else if ignored == doltdb.Ignore { + continue + } else { + return stagedChangedTables, unstagedChangedTables, fmt.Errorf("unrecognized ignore result value: %v", ignored) + } + + unstagedChangedTables[tableName] = true + } + } + + return stagedChangedTables, unstagedChangedTables, nil +} From d1c86adaf1342bf2efb01e4dee8b9ab957b03554 Mon Sep 17 00:00:00 2001 From: PavelSafronov Date: Fri, 7 Jul 2023 17:10:27 +0000 Subject: [PATCH 19/20] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/cherry-pick.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index d35cd0a27b5..7ae876793d5 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -17,12 +17,12 @@ package commands import ( "context" "fmt" + "strings" "github.com/dolthub/go-mysql-server/sql" "github.com/gocraft/dbr/v2" "github.com/gocraft/dbr/v2/dialect" "gopkg.in/src-d/go-errors.v1" - "strings" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" From e3b149f8933bfc3625c6dec4ae86999ab36cf2ef Mon Sep 17 00:00:00 2001 From: Pavel Safronov Date: Fri, 7 Jul 2023 14:13:48 -0700 Subject: [PATCH 20/20] PR feedback: update comment, remove debug statement. --- go/cmd/dolt/commands/cherry-pick.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 7ae876793d5..4dec0666d46 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -93,7 +93,7 @@ func (cmd CherryPickCmd) Exec(ctx context.Context, commandStr string, args []str defer closeFunc() } - // This command creates a commit, so we need user identity + // dolt_cherry_pick performs this check as well. Check performed early here to short circuit the operation. err = branch_control.CheckAccess(sqlCtx, branch_control.Permissions_Write) if err != nil { cli.Println(err.Error()) @@ -211,7 +211,7 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { query := "call dolt_cherry_pick('--abort')" - rows, err := GetRowsForSql(queryist, sqlCtx, query) + _, err := GetRowsForSql(queryist, sqlCtx, query) if err != nil { errorText := err.Error() switch errorText { @@ -221,7 +221,6 @@ func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { return err } } - cli.Printf("rows: %s\n", rows) return nil }