Merge pull request #6015 from dolthub/nicktobey/addcommand

Migrate `dolt add` to use Sql backend.

go/cmd/dolt/commands/add.go

@@ -15,8 +15,11 @@
 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"
@@ -40,6 +43,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"
@@ -59,40 +68,75 @@ func (cmd AddCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateAddArgParser()
 }
 
+// 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
+	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) {
+		write("-A")
+	}
+	if apr.Contains(cli.ForceFlag) {
+		write("-f")
+	}
+	for _, arg := range apr.Args {
+		write(arg)
+	}
+	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 {
 	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)
+	queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx)
 	if err != nil {
-		return HandleStageError(err)
+		cli.PrintErrln(errhand.VerboseErrorFromError(err))
+		return 1
+	}
+	if closeFunc != nil {
+		defer closeFunc()
 	}
 
-	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)
+	// 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
+	}
+
+	for _, tableName := range apr.Args {
+		if tableName != "." && !doltdb.IsValidTableName(tableName) {
+			return HandleVErrAndExitCode(errhand.BuildDError("'%s' is not a valid table name", tableName).Build(), nil)
 		}
 	}
 
-	err = dEnv.UpdateRoots(ctx, roots)
+	schema, rowIter, err := queryist.Query(sqlCtx, generateSql(apr))
 	if err != nil {
-		return HandleStageError(err)
+		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
@@ -128,23 +172,8 @@ 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()
 	}
 }
-
-func HandleDocTableVErrAndExitCode() int {
-	return HandleVErrAndExitCode(errhand.BuildDError("'%s' is not a valid table name", doltdb.DocTableName).Build(), nil)
-}

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()
 	}

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
 	}

go/cmd/dolt/dolt.go

@@ -27,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"
@@ -119,7 +120,7 @@ var doltSubCommands = []cli.Command{
 	&commands.Assist{},
 }
 
-var subCommandsUsingDEnv = []cli.Command{
+var commandsWithoutCliCtx = []cli.Command{
 	commands.InitCmd{},
 	commands.StatusCmd{},
 	commands.DiffCmd{},
@@ -135,8 +136,6 @@ var subCommandsUsingDEnv = []cli.Command{
 	commands.CheckoutCmd{},
 	commands.MergeCmd{},
 	cnfcmds.Commands,
-	commands.CherryPickCmd{},
-	commands.RevertCmd{},
 	commands.CloneCmd{},
 	commands.FetchCmd{},
 	commands.PullCmd{},
@@ -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
 		}
@@ -530,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
@@ -563,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
 
@@ -581,6 +580,17 @@ func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.Mult
 		targetEnv = mrEnv.GetEnv(useDb)
 	}
 
+	// 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" && 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.")
+		}, nil
+	}
+
 	// 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()

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 {

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
+}

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~

integration-tests/bats/sql-local-remote.bats

@@ -24,6 +24,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
 
@@ -132,3 +140,21 @@ teardown() {
     [ "$status" -eq 0 ]
     [[ "$output" =  $out ]] || false
 }
+@test "sql-local-remote: verify simple dolt add behavior." {
+    start_sql_server altDB
+    cd altDB
+
+    run dolt --verbose-engine-setup --user dolt sql -q "create table testtable (pk int PRIMARY KEY)"
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "starting remote mode" ]] || false
+
+    run dolt --verbose-engine-setup --user dolt add .
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "starting remote mode" ]] || false
+
+    stop_sql_server 1
+
+    staged=$(get_staged_tables)
+
+    [[ ! -z $(echo "$staged" | grep "testtable") ]] || false
+}