Adding support for deleting and renaming branches from SQL, along with additional test cases for dolt_branch.

2026-01-31 03:18:43 -06:00 · 2022-05-25 15:00:52 -07:00
parent 32e8866e0d
commit 02099d703c
5 changed files with 222 additions and 50 deletions
--- a/go/cmd/dolt/commands/branch.go
+++ b/go/cmd/dolt/commands/branch.go
@@ -220,7 +220,7 @@ func moveBranch(ctx context.Context, dEnv *env.DoltEnv, apr *argparser.ArgParseR
 	force := apr.Contains(forceFlag)
 	src := apr.Arg(0)
 	dest := apr.Arg(1)
-	err := actions.RenameBranch(ctx, dEnv, src, apr.Arg(1), force)
+	err := actions.RenameBranch(ctx, dEnv.DbData(), dEnv.Config, src, apr.Arg(1), force)

 	var verr errhand.VerboseError
 	if err != nil {
@@ -285,7 +285,7 @@ func handleDeleteBranches(ctx context.Context, dEnv *env.DoltEnv, apr *argparser
 	for i := 0; i < apr.NArg(); i++ {
 		brName := apr.Arg(i)

-		err := actions.DeleteBranch(ctx, dEnv, brName, actions.DeleteOptions{
+		err := actions.DeleteBranch(ctx, dEnv.DbData(), dEnv.Config, brName, actions.DeleteOptions{
 			Force:  force,
 			Remote: apr.Contains(remoteFlag),
 		})
--- a/go/libraries/doltcore/env/actions/branch.go
+++ b/go/libraries/doltcore/env/actions/branch.go
@@ -30,17 +30,17 @@ var ErrAlreadyExists = errors.New("already exists")
 var ErrCOBranchDelete = errors.New("attempted to delete checked out branch")
 var ErrUnmergedBranchDelete = errors.New("attempted to delete a branch that is not fully merged into its parent; use `-f` to force")

-func RenameBranch(ctx context.Context, dEnv *env.DoltEnv, oldBranch, newBranch string, force bool) error {
+func RenameBranch(ctx context.Context, dbData env.DbData, config *env.DoltCliConfig, oldBranch, newBranch string, force bool) error {
 	oldRef := ref.NewBranchRef(oldBranch)
 	newRef := ref.NewBranchRef(newBranch)

-	err := CopyBranch(ctx, dEnv, oldBranch, newBranch, force)
+	err := CopyBranchOnDB(ctx, dbData.Ddb, oldBranch, newBranch, force)
 	if err != nil {
 		return err
 	}

-	if ref.Equals(dEnv.RepoStateReader().CWBHeadRef(), oldRef) {
-		err = dEnv.RepoStateWriter().SetCWBHeadRef(ctx, ref.MarshalableRef{Ref: newRef})
+	if ref.Equals(dbData.Rsr.CWBHeadRef(), oldRef) {
+		err = dbData.Rsw.SetCWBHeadRef(ctx, ref.MarshalableRef{Ref: newRef})
 		if err != nil {
 			return err
 		}
@@ -59,13 +59,13 @@ func RenameBranch(ctx context.Context, dEnv *env.DoltEnv, oldBranch, newBranch s
 		// We always `force` here, because the CopyBranch up
 		// above created a new branch and it will have a
 		// working set.
-		err = dEnv.DoltDB.CopyWorkingSet(ctx, fromWSRef, toWSRef, true /* force */)
+		err = dbData.Ddb.CopyWorkingSet(ctx, fromWSRef, toWSRef, true /* force */)
 		if err != nil {
 			return err
 		}
 	}

-	return DeleteBranch(ctx, dEnv, oldBranch, DeleteOptions{Force: true})
+	return DeleteBranch(ctx, dbData, config, oldBranch, DeleteOptions{Force: true})
 }

 func CopyBranch(ctx context.Context, dEnv *env.DoltEnv, oldBranch, newBranch string, force bool) error {
@@ -111,7 +111,7 @@ type DeleteOptions struct {
 	Remote bool
 }

-func DeleteBranch(ctx context.Context, dEnv *env.DoltEnv, brName string, opts DeleteOptions) error {
+func DeleteBranch(ctx context.Context, dbData env.DbData, config *env.DoltCliConfig, brName string, opts DeleteOptions) error {
 	var dref ref.DoltRef
 	if opts.Remote {
 		var err error
@@ -121,16 +121,16 @@ func DeleteBranch(ctx context.Context, dEnv *env.DoltEnv, brName string, opts De
 		}
 	} else {
 		dref = ref.NewBranchRef(brName)
-		if ref.Equals(dEnv.RepoStateReader().CWBHeadRef(), dref) {
+		if ref.Equals(dbData.Rsr.CWBHeadRef(), dref) {
 			return ErrCOBranchDelete
 		}
 	}

-	return DeleteBranchOnDB(ctx, dEnv, dref, opts)
+	return DeleteBranchOnDB(ctx, dbData, config, dref, opts)
 }

-func DeleteBranchOnDB(ctx context.Context, dEnv *env.DoltEnv, dref ref.DoltRef, opts DeleteOptions) error {
-	ddb := dEnv.DoltDB
+func DeleteBranchOnDB(ctx context.Context, dbData env.DbData, config *env.DoltCliConfig, dref ref.DoltRef, opts DeleteOptions) error {
+	ddb := dbData.Ddb
 	hasRef, err := ddb.HasRef(ctx, dref)

 	if err != nil {
@@ -140,7 +140,7 @@ func DeleteBranchOnDB(ctx context.Context, dEnv *env.DoltEnv, dref ref.DoltRef,
 	}

 	if !opts.Force && !opts.Remote {
-		ms, err := doltdb.NewCommitSpec(env.GetDefaultInitBranch(dEnv.Config))
+		ms, err := doltdb.NewCommitSpec(env.GetDefaultInitBranch(config))
 		if err != nil {
 			return err
 		}
--- a/go/libraries/doltcore/sqle/dfunctions/dolt_branch.go
+++ b/go/libraries/doltcore/sqle/dfunctions/dolt_branch.go
@@ -28,6 +28,7 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
 	"github.com/dolthub/dolt/go/libraries/utils/argparser"
+	"github.com/dolthub/dolt/go/libraries/utils/filesys"
 )

 const DoltBranchFuncName = "dolt_branch"
@@ -91,50 +92,77 @@ func DoDoltBranch(ctx *sql.Context, args []string) (int, error) {

 	switch {
 	case apr.Contains(cli.CopyFlag):
-		err = makeACopyOfBranch(ctx, dbData, apr)
-		if err != nil {
-			return 1, err
-		}
+		err = copyBranch(ctx, dbData, apr)
 	case apr.Contains(cli.MoveFlag):
-		return 1, errors.New("Renaming a branch is not supported.")
-	case apr.Contains(cli.DeleteFlag):
-		return 1, errors.New("Deleting branches is not supported.")
-	case apr.Contains(cli.DeleteForceFlag):
-		return 1, errors.New("Deleting branches is not supported.")
+		err = renameBranch(ctx, dbData, apr)
+	case apr.Contains(cli.DeleteFlag), apr.Contains(cli.DeleteForceFlag):
+		err = deleteBranches(ctx, apr, dbData)
 	default:
-		// regular branch - create new branch
-		if apr.NArg() != 1 {
-			return 1, InvalidArgErr
-		}
-
-		branchName := apr.Arg(0)
-		if len(branchName) == 0 {
-			return 1, EmptyBranchNameErr
-		}
-
-		err = createNewBranch(ctx, dbData, branchName)
-		if err != nil {
-			return 1, err
-		}
+		err = createNewBranch(ctx, dbData, apr)
 	}

-	return 0, nil
-}
-
-func createNewBranch(ctx *sql.Context, dbData env.DbData, branchName string) error {
-	// Check if the branch already exists.
-	isBranch, err := actions.IsBranch(ctx, dbData.Ddb, branchName)
 	if err != nil {
-		return err
-	} else if isBranch {
-		return errors.New(fmt.Sprintf("fatal: A branch named '%s' already exists.", branchName))
+		return 1, err
+	} else {
+		return 0, nil
 	}
-
-	startPt := fmt.Sprintf("head")
-	return actions.CreateBranchWithStartPt(ctx, dbData, branchName, startPt, false)
 }

-func makeACopyOfBranch(ctx *sql.Context, dbData env.DbData, apr *argparser.ArgParseResults) error {
+func renameBranch(ctx *sql.Context, dbData env.DbData, apr *argparser.ArgParseResults) error {
+	if apr.NArg() != 2 {
+		return InvalidArgErr
+	}
+	oldBranchName, newBranchName := apr.Arg(0), apr.Arg(1)
+	if oldBranchName == "" || newBranchName == "" {
+		return EmptyBranchNameErr
+	}
+	force := apr.Contains(cli.ForceFlag)
+
+	return actions.RenameBranch(ctx, dbData, loadConfig(ctx), oldBranchName, newBranchName, force)
+}
+
+func deleteBranches(ctx *sql.Context, apr *argparser.ArgParseResults, dbData env.DbData) error {
+	if apr.NArg() == 0 {
+		return InvalidArgErr
+	}
+
+	for _, branchName := range apr.Args {
+		if len(branchName) == 0 {
+			return EmptyBranchNameErr
+		}
+		force := apr.Contains(cli.DeleteForceFlag) || apr.Contains(cli.ForceFlag)
+		err := actions.DeleteBranch(ctx, dbData, loadConfig(ctx), branchName, actions.DeleteOptions{
+			Force: force,
+		})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func loadConfig(ctx *sql.Context) *env.DoltCliConfig {
+	// When executing branch actions from SQL, we don't have access to a DoltEnv like we do from
+	// within the CLI. We can fake it here enough to get a DoltCliConfig, but we can't rely on the
+	// DoltEnv because tests and production will run with different settings (e.g. in-mem versus file).
+	dEnv := env.Load(ctx, env.GetCurrentUserHomeDir, filesys.LocalFS, doltdb.LocalDirDoltDB, "")
+	return dEnv.Config
+}
+
+func createNewBranch(ctx *sql.Context, dbData env.DbData, apr *argparser.ArgParseResults) error {
+	if apr.NArg() != 1 {
+		return InvalidArgErr
+	}
+
+	branchName := apr.Arg(0)
+	if len(branchName) == 0 {
+		return EmptyBranchNameErr
+	}
+
+	return actions.CreateBranchWithStartPt(ctx, dbData, branchName, "HEAD", apr.Contains(cli.ForceFlag))
+}
+
+func copyBranch(ctx *sql.Context, dbData env.DbData, apr *argparser.ArgParseResults) error {
 	if apr.NArg() != 2 {
 		return InvalidArgErr
 	}
--- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go
@@ -652,6 +652,12 @@ func TestDoltReset(t *testing.T) {
 	}
 }

+func TestDoltBranch(t *testing.T) {
+	for _, script := range DoltBranchScripts {
+		enginetest.TestScript(t, newDoltHarness(t), script)
+	}
+}
+
 // TestSingleTransactionScript is a convenience method for debugging a single transaction test. Unskip and set to the
 // desired test.
 func TestSingleTransactionScript(t *testing.T) {
--- a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
@@ -987,6 +987,144 @@ var MergeScripts = []queries.ScriptTest{
 	},
 }

+var DoltBranchScripts = []queries.ScriptTest{
+	{
+		Name: "Create branches from HEAD with dolt_branch procedure",
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:    "CALL DOLT_BRANCH('myNewBranch1')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				Query:    "SELECT COUNT(*) FROM DOLT_BRANCHES WHERE NAME='myNewBranch1';",
+				Expected: []sql.Row{{1}},
+			},
+			{
+				// Trying to recreate that branch fails without the force flag
+				Query:          "CALL DOLT_BRANCH('myNewBranch1')",
+				ExpectedErrStr: "fatal: A branch named 'myNewBranch1' already exists.",
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-f', 'myNewBranch1')",
+				Expected: []sql.Row{{0}},
+			},
+		},
+	},
+	{
+		Name: "Rename branches with dolt_branch procedure",
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:    "CALL DOLT_BRANCH('myNewBranch1')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('myNewBranch2')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				// Renaming to an existing name fails without the force flag
+				Query:          "CALL DOLT_BRANCH('-m', 'myNewBranch1', 'myNewBranch2')",
+				ExpectedErrStr: "already exists",
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-mf', 'myNewBranch1', 'myNewBranch2')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-m', 'myNewBranch2', 'myNewBranch3')",
+				Expected: []sql.Row{{0}},
+			},
+		},
+	},
+	{
+		Name: "Copy branches from other branches using dolt_branch procedure",
+		SetUpScript: []string{
+			"CALL DOLT_BRANCH('myNewBranch1')",
+		},
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:          "CALL DOLT_BRANCH('-c')",
+				ExpectedErrStr: "error: invalid usage",
+			},
+			{
+				Query:          "CALL DOLT_BRANCH('-c', 'myNewBranch1')",
+				ExpectedErrStr: "error: invalid usage",
+			},
+			{
+				Query:          "CALL DOLT_BRANCH('-c', 'myNewBranch2')",
+				ExpectedErrStr: "error: invalid usage",
+			},
+			{
+				Query:          "CALL DOLT_BRANCH('-c', '', '')",
+				ExpectedErrStr: "error: cannot branch empty string",
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-c', 'myNewBranch1', 'myNewBranch2')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				Query:    "SELECT COUNT(*) FROM DOLT_BRANCHES WHERE NAME='myNewBranch2';",
+				Expected: []sql.Row{{1}},
+			},
+			{
+				Query:          "CALL DOLT_BRANCH('-c', 'myNewBranch1', 'myNewBranch2')",
+				ExpectedErrStr: "fatal: A branch named 'myNewBranch2' already exists.",
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-cf', 'myNewBranch1', 'myNewBranch2')",
+				Expected: []sql.Row{{0}},
+			},
+		},
+	},
+	{
+		Name: "Delete branches with dolt_branch procedure",
+		SetUpScript: []string{
+			"CALL DOLT_BRANCH('myNewBranch1')",
+			"CALL DOLT_BRANCH('myNewBranch2')",
+			"CALL DOLT_BRANCH('myNewBranch3')",
+			"CALL DOLT_BRANCH('myNewBranchWithCommit')",
+			"CALL DOLT_CHECKOUT('myNewBranchWithCommit')",
+			"CALL DOLT_COMMIT('--allow-empty', '-am', 'empty commit')",
+			"CALL DOLT_CHECKOUT('main')",
+		},
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query:          "CALL DOLT_BRANCH('-d')",
+				ExpectedErrStr: "error: invalid usage",
+			},
+			{
+				Query:          "CALL DOLT_BRANCH('-d', '')",
+				ExpectedErrStr: "error: cannot branch empty string",
+			},
+			{
+				Query:          "CALL DOLT_BRANCH('-d', 'branchDoesNotExist')",
+				ExpectedErrStr: "branch not found",
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-d', 'myNewBranch1')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				Query:    "SELECT COUNT(*) FROM DOLT_BRANCHES WHERE NAME='myNewBranch1'",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-d', 'myNewBranch2', 'myNewBranch3')",
+				Expected: []sql.Row{{0}},
+			},
+			{
+				// Trying to delete a branch with unpushed changes fails without force option
+				Query:          "CALL DOLT_BRANCH('-d', 'myNewBranchWithCommit')",
+				ExpectedErrStr: "attempted to delete a branch that is not fully merged into its parent; use `-f` to force",
+			},
+			{
+				Query:    "CALL DOLT_BRANCH('-df', 'myNewBranchWithCommit')",
+				Expected: []sql.Row{{0}},
+			},
+		},
+	},
+}
+
 var DoltReset = []queries.ScriptTest{
 	{
 		Name: "CALL DOLT_RESET('--hard') should reset the merge state after uncommitted merge",