diff --git a/.github/workflows/ci-bats-unix.yaml b/.github/workflows/ci-bats-unix.yaml index d20f6f3e9b..0c9dd655d2 100644 --- a/.github/workflows/ci-bats-unix.yaml +++ b/.github/workflows/ci-bats-unix.yaml @@ -111,7 +111,7 @@ jobs: run: expect -v - name: Test all Unix env: - SQL_ENGINE: ${{ matrix.sql-engine }} + SQL_ENGINE: ${{ matrix.sql_engine }} PARQUET_RUNTIME_JAR: ${{ steps.parquet_cli.outputs.runtime_jar }} BATS_TEST_RETRIES: "3" run: | diff --git a/.github/workflows/ci-go-race-enginetests.yaml b/.github/workflows/ci-go-race-enginetests.yaml index e1fbd51e93..0a4bc5f871 100644 --- a/.github/workflows/ci-go-race-enginetests.yaml +++ b/.github/workflows/ci-go-race-enginetests.yaml @@ -29,6 +29,6 @@ jobs: - name: Test All working-directory: ./go run: | - DOLT_SKIP_PREPARED_ENGINETESTS=1 go test -v -race -timeout 30m github.com/dolthub/dolt/go/libraries/doltcore/sqle/enginetest + DOLT_SKIP_PREPARED_ENGINETESTS=1 go test -vet=off -v -race -timeout 30m github.com/dolthub/dolt/go/libraries/doltcore/sqle/enginetest env: DOLT_DEFAULT_BIN_FORMAT: ${{ matrix.dolt_fmt }} diff --git a/.github/workflows/ci-go-tests.yaml b/.github/workflows/ci-go-tests.yaml index 4e5d9cb740..c253075edd 100644 --- a/.github/workflows/ci-go-tests.yaml +++ b/.github/workflows/ci-go-tests.yaml @@ -44,12 +44,12 @@ jobs: if [ "$MATRIX_OS" == 'ubuntu-22.04' ] then if [[ "${file_arr[$i]}" != *enginetest* ]]; then - go test -timeout 45m -race "${file_arr[$i]}" + go test -vet=off -timeout 45m -race "${file_arr[$i]}" else echo "skipping enginetests for -race" fi else - go test -timeout 45m "${file_arr[$i]}" + go test -vet=off -timeout 45m "${file_arr[$i]}" fi succeeded=$(echo "$?") if [ "$succeeded" -ne 0 ]; then @@ -79,8 +79,8 @@ jobs: - name: Test All working-directory: ./go run: | - go test -timeout 30m ./libraries/doltcore/sqle/altertests - go test -timeout 30m ./libraries/doltcore/sqle/integration_test + go test -vet=off -timeout 30m ./libraries/doltcore/sqle/altertests + go test -vet=off -timeout 30m ./libraries/doltcore/sqle/integration_test env: MATRIX_OS: ${{ matrix.os }} DOLT_TEST_RUN_NON_RACE_TESTS: "true" diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 6257a230e0..bd13ef2ee8 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -77,6 +77,7 @@ func ParseAuthor(authorStr string) (string, string, error) { const ( AllowEmptyFlag = "allow-empty" + SkipEmptyFlag = "skip-empty" DateParam = "date" MessageArg = "message" AuthorParam = "author" @@ -140,7 +141,8 @@ var branchForceFlagDesc = "Reset {{.LessThan}}branchname{{.GreaterThan}} to {{.L func CreateCommitArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithMaxArgs("commit", 0) ap.SupportsString(MessageArg, "m", "msg", "Use the given {{.LessThan}}msg{{.GreaterThan}} as the commit message.") - ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety.") + ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety. Cannot be used with --skip-empty.") + ap.SupportsFlag(SkipEmptyFlag, "", "Only create a commit if there are staged changes. If no changes are staged, the call to commit is a no-op. Cannot be used with --allow-empty.") ap.SupportsString(DateParam, "", "date", "Specify the date used in the commit. If not specified the current system time is used.") ap.SupportsFlag(ForceFlag, "f", "Ignores any foreign key warnings and proceeds with the commit.") ap.SupportsString(AuthorParam, "", "author", "Specify an explicit author using the standard A U Thor {{.LessThan}}author@example.com{{.GreaterThan}} format.") @@ -403,3 +405,13 @@ func VerifyNoAwsParams(apr *argparser.ArgParseResults) error { return nil } + +// VerifyCommitArgs validates the arguments in |apr| for `dolt commit` and returns an error +// if any validation problems were encountered. +func VerifyCommitArgs(apr *argparser.ArgParseResults) error { + if apr.Contains(AllowEmptyFlag) && apr.Contains(SkipEmptyFlag) { + return fmt.Errorf("error: cannot use both --allow-empty and --skip-empty") + } + + return nil +} diff --git a/go/cmd/dolt/commands/add.go b/go/cmd/dolt/commands/add.go index ebbb266031..f7dd398fb3 100644 --- a/go/cmd/dolt/commands/add.go +++ b/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) -} diff --git a/go/cmd/dolt/commands/blame.go b/go/cmd/dolt/commands/blame.go index 2105f128fd..67dc94a360 100644 --- a/go/cmd/dolt/commands/blame.go +++ b/go/cmd/dolt/commands/blame.go @@ -17,15 +17,22 @@ package commands import ( "context" "fmt" + "regexp" + + "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" + "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" + ref2 "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/dolt/go/libraries/utils/argparser" + "github.com/dolthub/dolt/go/libraries/utils/iohelp" ) const ( - blameQueryTemplate = "SELECT * FROM dolt_blame_%s" + blameQueryTemplate = "SELECT * FROM dolt_blame_%s AS OF '%s'" ) var blameDocs = cli.CommandDocumentationContent{ @@ -54,10 +61,16 @@ func (cmd BlameCmd) Docs() *cli.CommandDocumentation { } func (cmd BlameCmd) ArgParser() *argparser.ArgParser { - ap := argparser.NewArgParserWithMaxArgs(cmd.Name(), 1) + ap := argparser.NewArgParserWithMaxArgs(cmd.Name(), 2) return ap } +func (cmd BlameCmd) RequiresRepo() bool { + return false +} + +var _ cli.RepoNotRequiredCommand = BlameCmd{} + // EventType returns the type of the event to log func (cmd BlameCmd) EventType() eventsapi.ClientEventType { return eventsapi.ClientEventType_BLAME @@ -84,11 +97,49 @@ func (cmd BlameCmd) Exec(ctx context.Context, commandStr string, args []string, help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, blameDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) - if apr.NArg() != 1 { + if apr.NArg() > 2 || apr.NArg() == 0 { usage() return 1 } - args = []string{"--" + QueryFlag, fmt.Sprintf(blameQueryTemplate, apr.Arg(0))} - return SqlCmd{}.Exec(ctx, "sql", args, dEnv, cliCtx) + queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) + if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) + return 1 + } + if closeFunc != nil { + defer closeFunc() + } + + var schema sql.Schema + var ri sql.RowIter + if apr.NArg() == 1 { + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(0), "HEAD")) + } else { + // validate input + ref := apr.Arg(0) + if !ref2.IsValidTagName(ref) && !doltdb.IsValidCommitHash(ref) && !isValidHeadRef(ref) { + iohelp.WriteLine(cli.CliOut, "Invalid reference provided") + return 1 + } + + schema, ri, err = queryist.Query(sqlCtx, fmt.Sprintf(blameQueryTemplate, apr.Arg(1), apr.Arg(0))) + } + if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) + return 1 + } + + err = engine.PrettyPrintResults(sqlCtx, engine.FormatTabular, schema, ri) + if err != nil { + iohelp.WriteLine(cli.CliOut, err.Error()) + return 1 + } + + return 0 +} + +func isValidHeadRef(s string) bool { + var refRegex = regexp.MustCompile(`(?i)^head[\~\^0-9]*$`) + return refRegex.MatchString(s) } 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() } diff --git a/go/cmd/dolt/commands/commit.go b/go/cmd/dolt/commands/commit.go index 7cf9cf1ae9..ad468cb0a4 100644 --- a/go/cmd/dolt/commands/commit.go +++ b/go/cmd/dolt/commands/commit.go @@ -77,41 +77,53 @@ func (cmd CommitCmd) ArgParser() *argparser.ArgParser { // Exec executes the command func (cmd CommitCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int { - res := performCommit(ctx, commandStr, args, dEnv) + res, skipped := performCommit(ctx, commandStr, args, dEnv) if res == 1 { return res } + if skipped { + iohelp.WriteLine(cli.CliOut, "Skipping empty commit") + return res + } + // if the commit was successful, print it out using the log command return LogCmd{}.Exec(ctx, "log", []string{"-n=1"}, dEnv, nil) } -func performCommit(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv) int { +// performCommit creates a new Dolt commit using the specified |commandStr| and |args| for the specified Dolt environment +// |dEnv|. The response is an integer status code indicating success or failure, as well as a boolean that indicates +// if the commit was skipped (e.g. because --skip-empty was specified as an argument). +func performCommit(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv) (int, bool) { ap := cli.CreateCommitArgParser() help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, commitDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) + if err := cli.VerifyCommitArgs(apr); err != nil { + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), help), false + } + allFlag := apr.Contains(cli.AllFlag) upperCaseAllFlag := apr.Contains(cli.UpperCaseAllFlag) if dEnv.IsLocked() { - return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help) + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help), false } roots, err := dEnv.Roots(ctx) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working root").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working root").AddCause(err).Build(), usage), false } if upperCaseAllFlag { roots, err = actions.StageAllTables(ctx, roots, true) if err != nil { - return handleCommitErr(ctx, dEnv, err, help) + return handleCommitErr(ctx, dEnv, err, help), false } } else if allFlag { roots, err = actions.StageModifiedAndDeletedTables(ctx, roots) if err != nil { - return handleCommitErr(ctx, dEnv, err, help) + return handleCommitErr(ctx, dEnv, err, help), false } } @@ -125,13 +137,13 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * } else { // This command creates a commit, so we need user identity if !cli.CheckUserNameAndEmail(dEnv) { - return 1 + return 1, false } name, email, err = env.GetNameAndEmail(dEnv.Config) } if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } msg, msgOk := apr.GetValue(cli.MessageArg) @@ -140,13 +152,13 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { commitMeta, cmErr := headCommit.GetCommitMeta(ctx) if cmErr != nil { - return handleCommitErr(ctx, dEnv, cmErr, usage) + return handleCommitErr(ctx, dEnv, cmErr, usage), false } amendStr = commitMeta.Description } msg, err = getCommitMessageFromEditor(ctx, dEnv, "", amendStr, false) if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } } @@ -156,7 +168,7 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * t, err = cli.ParseDate(commitTimeStr) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("error: invalid date").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("error: invalid date").AddCause(err).Build(), usage), false } } @@ -172,23 +184,23 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * newRoots, err := actions.ResetSoftToRef(ctx, dEnv.DbData(), "HEAD~1") if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } ws, err := dEnv.WorkingSet(ctx) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage), false } prevHash, err := ws.HashOf() if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage), false } var mergeParentCommits []*doltdb.Commit @@ -202,6 +214,7 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * Message: msg, Date: t, AllowEmpty: apr.Contains(cli.AllowEmptyFlag) || apr.Contains(cli.AmendFlag), + SkipEmpty: apr.Contains(cli.SkipEmptyFlag), Force: apr.Contains(cli.ForceFlag), Name: name, Email: email, @@ -210,20 +223,26 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { newRoots, errRes := actions.ResetSoftToRef(ctx, dEnv.DbData(), headHash.String()) if errRes != nil { - return handleResetError(errRes, usage) + return handleResetError(errRes, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false + } + + // If no error was reported and there is no pending commit, then no commit was created, likely + // because of --skip-empty, so return a success code, but indicate that no commit was created. + if pendingCommit == nil { + return 0, true } headRef, err := dEnv.RepoStateReader().CWBHeadRef() if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } _, err = dEnv.DoltDB.CommitWithWorkingSet( ctx, @@ -239,18 +258,18 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { newRoots, errRes := actions.ResetSoftToRef(ctx, dEnv.DbData(), headHash.String()) if errRes != nil { - return handleResetError(errRes, usage) + return handleResetError(errRes, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't commit").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't commit").AddCause(err).Build(), usage), false } - return 0 + return 0, false } func handleCommitErr(ctx context.Context, dEnv *env.DoltEnv, err error, usage cli.UsagePrinter) int { diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index 93651de708..bee68abe10 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -580,8 +580,8 @@ func executeMergeAndCommit(ctx context.Context, dEnv *env.DoltEnv, spec *merge.M author := fmt.Sprintf("%s <%s>", spec.Name, spec.Email) - res := performCommit(ctx, "commit", []string{"-m", msg, "--author", author}, dEnv) - if res != 0 { + res, skipped := performCommit(ctx, "commit", []string{"-m", msg, "--author", author}, dEnv) + if res != 0 || skipped { return nil, fmt.Errorf("dolt commit failed after merging") } diff --git a/go/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 8e06bac264..6499b7f3d0 100644 --- a/go/cmd/dolt/dolt.go +++ b/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" @@ -60,7 +61,7 @@ import ( ) const ( - Version = "1.2.4" + Version = "1.3.0" ) var dumpDocsCommand = &commands.DumpDocsCmd{} @@ -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() diff --git a/go/go.mod b/go/go.mod index cacb4f5333..a8b1bceb0a 100755 --- a/go/go.mod +++ b/go/go.mod @@ -15,7 +15,7 @@ require ( github.com/dolthub/fslock v0.0.3 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 - github.com/dolthub/vitess v0.0.0-20230531182027-fe0363298c52 + github.com/dolthub/vitess v0.0.0-20230605213213-cbc62ca5fb39 github.com/dustin/go-humanize v1.0.0 github.com/fatih/color v1.13.0 github.com/flynn-archive/go-shlex v0.0.0-20150515145356-3f9db97f8568 @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.15.1-0.20230602200348-162f21795cab + github.com/dolthub/go-mysql-server v0.15.1-0.20230606163443-3036c6d7fff7 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index 1c0a4822d1..85aab3613b 100755 --- a/go/go.sum +++ b/go/go.sum @@ -168,8 +168,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.15.1-0.20230602200348-162f21795cab h1:+xeaazQ4jDIx0+Fo7MdmEfg/I5xjuDrO576zdfnGjlA= -github.com/dolthub/go-mysql-server v0.15.1-0.20230602200348-162f21795cab/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= +github.com/dolthub/go-mysql-server v0.15.1-0.20230606163443-3036c6d7fff7 h1:Tpl0xYr2MqfZhENtHr+H166D41nLRpL+XPchLhRHcwQ= +github.com/dolthub/go-mysql-server v0.15.1-0.20230606163443-3036c6d7fff7/go.mod h1:E1STXoY8HvbrOWwFOhHpMfIjgcjboBhFtSpRxPgiBOw= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= @@ -180,8 +180,8 @@ github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9X github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY= github.com/dolthub/swiss v0.1.0 h1:EaGQct3AqeP/MjASHLiH6i4TAmgbG/c4rA6a1bzCOPc= github.com/dolthub/swiss v0.1.0/go.mod h1:BeucyB08Vb1G9tumVN3Vp/pyY4AMUnr9p7Rz7wJ7kAQ= -github.com/dolthub/vitess v0.0.0-20230531182027-fe0363298c52 h1:MGNJQpIOGRxtcKEsmbuMXFhMIJdlfxeeBaDZjbI6ZAg= -github.com/dolthub/vitess v0.0.0-20230531182027-fe0363298c52/go.mod h1:IyoysiiOzrIs7QsEHC+yVF0yRQ6W70GXyCXqtI2vVTs= +github.com/dolthub/vitess v0.0.0-20230605213213-cbc62ca5fb39 h1:T1oXYVAZrpANwuyStC/SZGw4cYmPFMwq6n6KvK4U6Mw= +github.com/dolthub/vitess v0.0.0-20230605213213-cbc62ca5fb39/go.mod h1:IyoysiiOzrIs7QsEHC+yVF0yRQ6W70GXyCXqtI2vVTs= github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= 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 { diff --git a/go/libraries/doltcore/env/actions/commit.go b/go/libraries/doltcore/env/actions/commit.go index f348f71dc3..3fda0d3d9b 100644 --- a/go/libraries/doltcore/env/actions/commit.go +++ b/go/libraries/doltcore/env/actions/commit.go @@ -27,6 +27,7 @@ type CommitStagedProps struct { Message string Date time.Time AllowEmpty bool + SkipEmpty bool Amend bool Force bool Name string @@ -62,6 +63,10 @@ func GetCommitStaged( isEmpty := len(staged) == 0 allowEmpty := ws.MergeActive() || props.AllowEmpty || props.Amend + + if isEmpty && props.SkipEmpty { + return nil, nil + } if isEmpty && !allowEmpty { return nil, NothingStaged{notStaged} } diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go index ebc50f3236..9470b4de95 100644 --- a/go/libraries/doltcore/merge/merge_prolly_rows.go +++ b/go/libraries/doltcore/merge/merge_prolly_rows.go @@ -25,7 +25,6 @@ import ( "github.com/dolthub/go-mysql-server/memory" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/analyzer" - "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/planbuilder" "github.com/dolthub/go-mysql-server/sql/transform" errorkinds "gopkg.in/src-d/go-errors.v1" @@ -405,7 +404,6 @@ func (cv checkValidator) validateDiff(ctx *sql.Context, diff tree.ThreeWayDiff) var valueTuple val.Tuple var valueDesc val.TupleDesc - switch diff.Op { case tree.DiffOpLeftDelete, tree.DiffOpRightDelete, tree.DiffOpConvergentDelete: // no need to validate check constraints for deletes @@ -439,7 +437,7 @@ func (cv checkValidator) validateDiff(ctx *sql.Context, diff tree.ThreeWayDiff) newTuple = val.NewTuple(cv.valueMerger.syncPool, newTupleBytes...) } - row, err := cv.buildRow(ctx, diff.Key, newTuple) + row, err := buildRow(ctx, diff.Key, newTuple, cv.sch, cv.tableMerger) if err != nil { return 0, err } @@ -482,10 +480,10 @@ func (cv checkValidator) insertArtifact(ctx context.Context, key, value val.Tupl } // buildRow takes the |key| and |value| tuple and returns a new sql.Row, along with any errors encountered. -func (cv checkValidator) buildRow(ctx *sql.Context, key, value val.Tuple) (sql.Row, error) { - pkCols := cv.sch.GetPKCols() - valueCols := cv.sch.GetNonPKCols() - allCols := cv.sch.GetAllCols() +func buildRow(ctx *sql.Context, key, value val.Tuple, sch schema.Schema, tableMerger *TableMerger) (sql.Row, error) { + pkCols := sch.GetPKCols() + valueCols := sch.GetNonPKCols() + allCols := sch.GetAllCols() // When we parse and resolve the check constraint expression with planbuilder, it leaves row position 0 // for the expression itself, so we add an empty spot in index 0 of our row to account for that to make sure @@ -494,10 +492,10 @@ func (cv checkValidator) buildRow(ctx *sql.Context, key, value val.Tuple) (sql.R // Skip adding the key tuple if we're working with a keyless table, since the table row data is // always all contained in the value tuple for keyless tables. - if !cv.valueMerger.keyless { - keyDesc := cv.sch.GetKeyDescriptor() + if !schema.IsKeyless(sch) { + keyDesc := sch.GetKeyDescriptor() for i := range keyDesc.Types { - value, err := index.GetField(ctx, keyDesc, i, key, cv.tableMerger.ns) + value, err := index.GetField(ctx, keyDesc, i, key, tableMerger.ns) if err != nil { return nil, err } @@ -508,15 +506,15 @@ func (cv checkValidator) buildRow(ctx *sql.Context, key, value val.Tuple) (sql.R } valueColIndex := 0 - valueDescriptor := cv.sch.GetValueDescriptor() + valueDescriptor := sch.GetValueDescriptor() for valueTupleIndex := range valueDescriptor.Types { // Skip processing the first value in the value tuple for keyless tables, since that field // always holds the cardinality of the row and shouldn't be passed in to an expression. - if cv.valueMerger.keyless && valueTupleIndex == 0 { + if schema.IsKeyless(sch) && valueTupleIndex == 0 { continue } - value, err := index.GetField(ctx, valueDescriptor, valueTupleIndex, value, cv.tableMerger.ns) + value, err := index.GetField(ctx, valueDescriptor, valueTupleIndex, value, tableMerger.ns) if err != nil { return nil, err } @@ -932,7 +930,7 @@ func (m *primaryMerger) merge(ctx *sql.Context, diff tree.ThreeWayDiff, sourceSc return fmt.Errorf("cannot merge keyless tables with reordered columns") } } else { - tempTupleValue, err := remapTupleWithColumnDefaults(ctx, diff.Right, sourceSch.GetValueDescriptor(), + tempTupleValue, err := remapTupleWithColumnDefaults(ctx, diff.Key, diff.Right, sourceSch.GetValueDescriptor(), m.valueMerger.rightMapping, m.tableMerger, m.finalSch, m.valueMerger.syncPool) if err != nil { return err @@ -1103,7 +1101,7 @@ func remapTuple(tuple val.Tuple, desc val.TupleDesc, mapping val.OrdinalMapping) // currently being merged and associated node store. |mergedSch| is the new schema of the table and is used to look up // column default values to apply to any existing rows when a new column is added as part of a merge. |pool| is used to // to allocate memory for the new tuple. A pointer to the new tuple data is returned, along with any error encountered. -func remapTupleWithColumnDefaults(ctx *sql.Context, tuple val.Tuple, tupleDesc val.TupleDesc, mapping val.OrdinalMapping, tm *TableMerger, mergedSch schema.Schema, pool pool.BuffPool) (val.Tuple, error) { +func remapTupleWithColumnDefaults(ctx *sql.Context, keyTuple, valueTuple val.Tuple, tupleDesc val.TupleDesc, mapping val.OrdinalMapping, tm *TableMerger, mergedSch schema.Schema, pool pool.BuffPool) (val.Tuple, error) { tb := val.NewTupleBuilder(mergedSch.GetValueDescriptor()) for to, from := range mapping { @@ -1113,22 +1111,21 @@ func remapTupleWithColumnDefaults(ctx *sql.Context, tuple val.Tuple, tupleDesc v col := mergedSch.GetNonPKCols().GetByIndex(to) if col.Default != "" { // TODO: Not great to reparse the expression for every single row... need to cache this - defaultValue, err := parse.StringToColumnDefaultValue(ctx, col.Default) + expression, err := resolveExpression(ctx, col.Default, mergedSch, tm.name) if err != nil { return nil, err } - // TODO: We can currently only handle column default values that only use literal - // values. Any expressions that need to be resolved (e.g. column references, - // functions) need the analyzer invoked on them before we can Eval() them. - // So, instead of potentially creating inconsistent data and not applying the - // correct default value that customers are expecting, throw an error and alert - // customers that they need to manually apply the alter to update existing rows. - if !defaultValue.Expression.Resolved() { - return nil, ErrUnableToMergeColumnDefaultValue.New(defaultValue, tm.name) + if !expression.Resolved() { + return nil, ErrUnableToMergeColumnDefaultValue.New(col.Default, tm.name) } - value, err = defaultValue.Expression.Eval(ctx, nil) + row, err := buildRow(ctx, keyTuple, valueTuple, mergedSch, tm) + if err != nil { + return nil, err + } + + value, err = expression.Eval(ctx, row) if err != nil { return nil, err } @@ -1142,7 +1139,7 @@ func remapTupleWithColumnDefaults(ctx *sql.Context, tuple val.Tuple, tupleDesc v } } } else { - tb.PutRaw(to, tupleDesc.GetField(from, tuple)) + tb.PutRaw(to, tupleDesc.GetField(from, valueTuple)) } } return tb.Build(pool), nil @@ -1284,7 +1281,7 @@ func migrateDataToMergedSchema(ctx *sql.Context, tm *TableMerger, vm *valueMerge return err } - newValueTuple, err := remapTupleWithColumnDefaults(ctx, valueTuple, valueDescriptor, vm.leftMapping, tm, mergedSch, vm.syncPool) + newValueTuple, err := remapTupleWithColumnDefaults(ctx, keyTuple, valueTuple, valueDescriptor, vm.leftMapping, tm, mergedSch, vm.syncPool) if err != nil { return err } diff --git a/go/libraries/doltcore/merge/violations_unique_prolly.go b/go/libraries/doltcore/merge/violations_unique_prolly.go index 0117c63ad1..0742b34ae1 100644 --- a/go/libraries/doltcore/merge/violations_unique_prolly.go +++ b/go/libraries/doltcore/merge/violations_unique_prolly.go @@ -163,7 +163,7 @@ func (m NullViolationMeta) ToString(ctx *sql.Context) (string, error) { // CheckCVMeta holds metadata describing a check constraint violation. type CheckCVMeta struct { Name string `json:"Name"` - Expression string `jason:"Expression"` + Expression string `json:"Expression"` } var _ types.JSONValue = CheckCVMeta{} diff --git a/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go b/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go index 5d111b7e6f..2bcbd6e98c 100644 --- a/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go +++ b/go/libraries/doltcore/sqle/binlogreplication/binlog_replication_test.go @@ -454,7 +454,7 @@ func TestCharsetsAndCollations(t *testing.T) { require.NoError(t, rows.Close()) // Test that we get an error for unsupported charsets/collations - primaryDatabase.MustExec("CREATE TABLE t2 (pk int primary key, c1 varchar(255) COLLATE utf16_german2_ci);") + primaryDatabase.MustExec("CREATE TABLE t2 (pk int primary key, c1 varchar(255) COLLATE utf8mb4_bg_0900_as_cs);") waitForReplicaToCatchUp(t) replicaDatabase.MustExec("use db01;") rows, err = replicaDatabase.Queryx("SHOW TABLES WHERE Tables_in_db01 like 't2';") @@ -467,7 +467,7 @@ func TestCharsetsAndCollations(t *testing.T) { row = convertByteArraysToStrings(readNextRow(t, rows)) require.Equal(t, "1105", row["Last_SQL_Errno"]) require.NotEmpty(t, row["Last_SQL_Error_Timestamp"]) - require.Contains(t, row["Last_SQL_Error"], "The collation `utf16_german2_ci` has not yet been implemented") + require.Contains(t, row["Last_SQL_Error"], "The collation `utf8mb4_bg_0900_as_cs` has not yet been implemented") require.False(t, rows.Next()) require.NoError(t, rows.Close()) } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go index c73d138f4c..b5d691b638 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go @@ -93,7 +93,8 @@ func doDoltCherryPick(ctx *sql.Context, args []string) (string, error) { return "", fmt.Errorf("dolt add failed") } - return doDoltCommit(ctx, []string{"-m", commitMsg}) + commitHash, _, err := doDoltCommit(ctx, []string{"-m", commitMsg}) + return commitHash, err } // cherryPick checks that the current working set is clean, verifies the cherry-pick commit is not a merge commit diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go b/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go index 6699687743..72fd7825cd 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go @@ -32,51 +32,65 @@ var hashType = types.MustCreateString(query.Type_TEXT, 32, sql.Collation_ascii_b // doltCommit is the stored procedure version for the CLI command `dolt commit`. func doltCommit(ctx *sql.Context, args ...string) (sql.RowIter, error) { - res, err := doDoltCommit(ctx, args) + commitHash, skipped, err := doDoltCommit(ctx, args) if err != nil { return nil, err } - return rowToIter(res), nil + if skipped { + return nil, nil + } + return rowToIter(commitHash), nil } // doltCommitHashOut is the stored procedure version for the CLI function `commit`. The first parameter is the variable // to set the hash of. func doltCommitHashOut(ctx *sql.Context, outHash *string, args ...string) (sql.RowIter, error) { - res, err := doDoltCommit(ctx, args) + commitHash, skipped, err := doDoltCommit(ctx, args) if err != nil { return nil, err } - *outHash = res - return rowToIter(res), nil + if skipped { + return nil, nil + } + + *outHash = commitHash + return rowToIter(commitHash), nil } -func doDoltCommit(ctx *sql.Context, args []string) (string, error) { +// doDoltCommit creates a dolt commit using the specified command line |args| provided. The response is the commit hash +// of the new commit (or the empty string if the commit was skipped), a boolean that indicates if creating the commit +// was skipped (e.g. due to --skip-empty), and an error describing any error encountered. +func doDoltCommit(ctx *sql.Context, args []string) (string, bool, error) { if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil { - return "", err + return "", false, err } // Get the information for the sql context. dbName := ctx.GetCurrentDatabase() apr, err := cli.CreateCommitArgParser().Parse(args) if err != nil { - return "", err + return "", false, err + } + + if err := cli.VerifyCommitArgs(apr); err != nil { + return "", false, err } dSess := dsess.DSessFromSess(ctx.Session) roots, ok := dSess.GetRoots(ctx, dbName) if !ok { - return "", fmt.Errorf("Could not load database %s", dbName) + return "", false, fmt.Errorf("Could not load database %s", dbName) } if apr.Contains(cli.UpperCaseAllFlag) { roots, err = actions.StageAllTables(ctx, roots, true) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } else if apr.Contains(cli.AllFlag) { roots, err = actions.StageModifiedAndDeletedTables(ctx, roots) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } @@ -84,7 +98,7 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { name, email, err = cli.ParseAuthor(authorStr) if err != nil { - return "", err + return "", false, err } } else { // In SQL mode, use the current SQL user as the commit author, instead of the `dolt config` configured values. @@ -100,15 +114,15 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { if amend { commit, err := dSess.GetHeadCommit(ctx, dbName) if err != nil { - return "", err + return "", false, err } commitMeta, err := commit.GetCommitMeta(ctx) if err != nil { - return "", err + return "", false, err } msg = commitMeta.Description } else { - return "", fmt.Errorf("Must provide commit message.") + return "", false, fmt.Errorf("Must provide commit message.") } } @@ -118,7 +132,7 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { t, err = cli.ParseDate(commitTimeStr) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } @@ -126,31 +140,34 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { Message: msg, Date: t, AllowEmpty: apr.Contains(cli.AllowEmptyFlag), + SkipEmpty: apr.Contains(cli.SkipEmptyFlag), Amend: amend, Force: apr.Contains(cli.ForceFlag), Name: name, Email: email, }) if err != nil { - return "", err + return "", false, err } // Nothing to commit, and we didn't pass --allowEmpty - if pendingCommit == nil { - return "", errors.New("nothing to commit") + if pendingCommit == nil && apr.Contains(cli.SkipEmptyFlag) { + return "", true, nil + } else if pendingCommit == nil { + return "", false, errors.New("nothing to commit") } newCommit, err := dSess.DoltCommit(ctx, dbName, dSess.GetTransaction(), pendingCommit) if err != nil { - return "", err + return "", false, err } h, err := newCommit.HashOf() if err != nil { - return "", err + return "", false, err } - return h.String(), nil + return h.String(), false, nil } func getDoltArgs(ctx *sql.Context, row sql.Row, children []sql.Expression) ([]string, error) { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index a587de2328..b619fe1a13 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -222,7 +222,7 @@ func performMerge(ctx *sql.Context, sess *dsess.DoltSession, roots doltdb.Roots, if !noCommit { author := fmt.Sprintf("%s <%s>", spec.Name, spec.Email) - _, err = doDoltCommit(ctx, []string{"-m", msg, "--author", author}) + _, _, err = doDoltCommit(ctx, []string{"-m", msg, "--author", author}) if err != nil { return ws, noConflictsOrViolations, threeWayMerge, fmt.Errorf("dolt_commit failed") } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go b/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go index f930a6fb82..4124fdee75 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go @@ -128,7 +128,7 @@ func doDoltRevert(ctx *sql.Context, args []string) (int, error) { if err != nil { return 1, err } - _, err = doDoltCommit(ctx, commitArgs) + _, _, err = doDoltCommit(ctx, commitArgs) if err != nil { return 1, err } diff --git a/go/libraries/doltcore/sqle/dsess/transactions.go b/go/libraries/doltcore/sqle/dsess/transactions.go index dd72525516..8a26994156 100644 --- a/go/libraries/doltcore/sqle/dsess/transactions.go +++ b/go/libraries/doltcore/sqle/dsess/transactions.go @@ -16,6 +16,7 @@ package dsess import ( "context" + "encoding/json" "errors" "fmt" "strings" @@ -27,10 +28,12 @@ import ( "github.com/sirupsen/logrus" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/dolt/go/store/hash" + "github.com/dolthub/dolt/go/store/prolly" ) const ( @@ -596,12 +599,101 @@ func (tx *DoltTransaction) validateWorkingSetForCommit(ctx *sql.Context, working // TODO: We need to add more granularity in terms of what types of constraint violations can be committed. For example, // in the case of foreign_key_checks=0 you should be able to commit foreign key violations. if forceTransactionCommit.(int8) != 1 { + badTbls, err := workingRoot.TablesWithConstraintViolations(ctx) + if err != nil { + return err + } + + violations := make([]string, len(badTbls)) + for i, name := range badTbls { + tbl, _, err := workingRoot.GetTable(ctx, name) + if err != nil { + return err + } + + artIdx, err := tbl.GetArtifacts(ctx) + if err != nil { + return err + } + + m := durable.ProllyMapFromArtifactIndex(artIdx) + itr, err := m.IterAllCVs(ctx) + + for { + art, err := itr.Next(ctx) + if err != nil { + break + } + + var meta prolly.ConstraintViolationMeta + err = json.Unmarshal(art.Metadata, &meta) + if err != nil { + return err + } + + s := "" + switch art.ArtType { + case prolly.ArtifactTypeForeignKeyViol: + var m merge.FkCVMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + s = fmt.Sprintf("\n"+ + "Type: Foreign Key Constraint Violation\n"+ + "\tForeignKey: %s,\n"+ + "\tTable: %s,\n"+ + "\tReferencedTable: %s,\n"+ + "\tIndex: %s,\n"+ + "\tReferencedIndex: %s", m.ForeignKey, m.Table, m.ReferencedIndex, m.Index, m.ReferencedIndex) + + case prolly.ArtifactTypeUniqueKeyViol: + var m merge.UniqCVMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + s = fmt.Sprintf("\n"+ + "Type: Unique Key Constraint Violation,\n"+ + "\tName: %s,\n"+ + "\tColumns: %v", m.Name, m.Columns) + + case prolly.ArtifactTypeNullViol: + var m merge.NullViolationMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + s = fmt.Sprintf("\n"+ + "Type: Null Constraint Violation,\n"+ + "\tColumns: %v", m.Columns) + + case prolly.ArtifactTypeChkConsViol: + var m merge.CheckCVMeta + err = json.Unmarshal(meta.VInfo, &m) + if err != nil { + return err + } + s = fmt.Sprintf("\n"+ + "Type: Check Constraint Violation,\n"+ + "\tName: %s,\n"+ + "\tExpression: %v", m.Name, m.Expression) + } + if err != nil { + return err + } + + violations[i] = s + } + } + rollbackErr := tx.rollback(ctx) if rollbackErr != nil { return rollbackErr } - return ErrUnresolvedConstraintViolationsCommit + return fmt.Errorf("%s\n"+ + "Constraint violations: %s", ErrUnresolvedConstraintViolationsCommit, strings.Join(violations, ", ")) } } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 0549137334..f5ce1475b9 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -1141,6 +1141,7 @@ func TestLoadData(t *testing.T) { } func TestLoadDataErrors(t *testing.T) { + t.Skip() h := newDoltHarness(t) defer h.Close() enginetest.TestLoadDataErrors(t, h) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 18d3e9272e..d330b2f2b3 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4285,30 +4285,55 @@ var ThreeWayMergeWithSchemaChangeTestScripts = []MergeScriptTest{ }, }, { - // TODO: We can currently only support literal default values. Supporting column references and functions - // requires getting the analyzer involved to resolve references. Name: "adding a column with a non-literal default value", AncSetUpScript: []string{ - "CREATE table t (pk int primary key);", - "INSERT into t values (1);", + "CREATE table t (pk varchar(100) primary key);", + "INSERT into t values ('1');", }, RightSetUpScript: []string{ - "alter table t add column c1 varchar(100) default (CONCAT('h','e','l','l','o'));", - "insert into t values (2, 'hi');", + "alter table t add column c1 varchar(100) default (CONCAT(pk, 'h','e','l','l','o'));", + "insert into t values ('2', 'hi');", "alter table t add index idx1 (c1, pk);", }, LeftSetUpScript: []string{ - "insert into t values (3);", + "insert into t values ('3');", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "call dolt_merge('right');", - ExpectedErr: merge.ErrUnableToMergeColumnDefaultValue, + Query: "call dolt_merge('right');", + Expected: []sql.Row{{0, 0}}, }, { - Skip: true, Query: "select * from t;", - Expected: []sql.Row{{1, "hello"}, {2, "hi"}, {3, "hello"}}, + Expected: []sql.Row{{"1", "1hello"}, {"2", "hi"}, {"3", "3hello"}}, + }, + }, + }, + { + // Tests that column default expressions are correctly evaluated when the left-side schema + // has changed and the right row needs to be mapped to the new left-side schema + Name: "right-side adds a column with a default value, left-side drops a column", + AncSetUpScript: []string{ + "CREATE table t (pk int primary key, c1 varchar(100), c2 varchar(100));", + "INSERT into t values ('1', 'BAD', 'hello');", + }, + RightSetUpScript: []string{ + "alter table t add column c3 varchar(100) default (CONCAT(c2, 'h','e','l','l','o'));", + "insert into t values ('2', 'BAD', 'hello', 'hi');", + "alter table t add index idx1 (c1, pk);", + }, + LeftSetUpScript: []string{ + "insert into t values ('3', 'BAD', 'hello');", + "alter table t drop column c1;", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('right');", + Expected: []sql.Row{{0, 0}}, + }, + { + Query: "select * from t;", + Expected: []sql.Row{{1, "hello", "hellohello"}, {2, "hello", "hi"}, {3, "hello", "hellohello"}}, }, }, }, diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go index 5796109543..57a9e9cf95 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go @@ -1995,7 +1995,14 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + + "Constraint violations: \n" + + "Type: Foreign Key Constraint Violation\n" + + "\tForeignKey: 0050p5ek,\n" + + "\tTable: child,\n" + + "\tReferencedTable: ,\n" + + "\tIndex: parent_fk,\n" + + "\tReferencedIndex: ", }, { Query: "/* client b */ INSERT INTO child VALUES (1, 1);", @@ -2024,7 +2031,11 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + + "Constraint violations: \n" + + "Type: Unique Key Constraint Violation,\n" + + "\tName: col1,\n" + + "\tColumns: [col1]", }, { Query: "/* client a */ SELECT * from DOLT_CONSTRAINT_VIOLATIONS;", @@ -2107,7 +2118,14 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ ExpectedErrStr: "Committing this transaction resulted in a working set with constraint violations, transaction rolled back. " + "This constraint violation may be the result of a previous merge or the result of transaction sequencing. " + "Constraint violations from a merge can be resolved using the dolt_constraint_violations table before committing the transaction. " + - "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.", + "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + + "Constraint violations: \n" + + "Type: Foreign Key Constraint Violation\n" + + "\tForeignKey: fk_name,\n" + + "\tTable: child,\n" + + "\tReferencedTable: v1,\n" + + "\tIndex: v1,\n" + + "\tReferencedIndex: v1", }, }, }, 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 diff --git a/integration-tests/bats/blame.bats b/integration-tests/bats/blame.bats index 2bcc91233f..70320875be 100644 --- a/integration-tests/bats/blame.bats +++ b/integration-tests/bats/blame.bats @@ -50,55 +50,54 @@ SQL [[ "$output" =~ "usage" ]] || false } +@test "blame: too many arguments shows usage" { + run dolt blame HEAD blame_test foo + [ "$status" -eq 1 ] + [[ "$output" =~ "blame has too many positional arguments" ]] || false +} + @test "blame: annotates a small table with simple history" { # should be the same as dolt blame HEAD blame_test run dolt blame blame_test [ "$status" -eq 0 ] - # TODO: Make these assertions better - [[ "$output" =~ "Thomas Foolery" ]] || false - [[ "$output" =~ "create blame_test table" ]] || false + [[ "${lines[1]}" =~ "pk".*"commit".*"commit_date".*"committer".*"email".*"message" ]] || false + [[ "$output" =~ "1 |".+"|".+"| Thomas Foolery, | bats-1@email.fake | create blame_test table |" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "Harry Wombat" ]] || false - [[ "$output" =~ "replace richard" ]] || false - [[ "$output" =~ "Johnny Moolah" ]] || false - [[ "$output" =~ "add more people" ]] || false + [[ "$output" =~ "2 |".+"|".+"| Harry Wombat, | bats-3@email.fake | replace richard with harry |" ]] || false + [[ "$output" =~ "3 |".+"|".+"| Johnny Moolah, | bats-4@email.fake | add more people to blame_test |" ]] || false + [[ "$output" =~ "4 |".+"|".+"| Johnny Moolah, | bats-4@email.fake | add more people to blame_test |" ]] || false } @test "blame: blames HEAD when commit ref omitted" { run dolt blame blame_test [ "$status" -eq 0 ] - [[ "$output" =~ "Thomas Foolery" ]] || false - [[ "$output" =~ "create blame_test table" ]] || false + + [[ "$output" =~ "1".*"Thomas Foolery".*"create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "Harry Wombat" ]] || false - [[ "$output" =~ "replace richard" ]] || false - [[ "$output" =~ "Johnny Moolah" ]] || false - [[ "$output" =~ "add more people" ]] || false + [[ "$output" =~ "2".*"Harry Wombat".*"replace richard" ]] || false + [[ "$output" =~ "3".*"Johnny Moolah".*"add more people" ]] || false + [[ "$output" =~ "4".*"Johnny Moolah".*"add more people" ]] || false } @test "blame: works with HEAD as the commit ref" { - skip "SQL views do no support AS OF queries" - run dolt blame HEAD blame_test [ "$status" -eq 0 ] - [[ "$output" =~ "Thomas Foolery" ]] || false - [[ "$output" =~ "create blame_test table" ]] || false + + [[ "$output" =~ "1".*"Thomas Foolery".*"create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false [[ ! "$output" =~ "add richard to blame_test" ]] || false - [[ "$output" =~ "Harry Wombat" ]] || false - [[ "$output" =~ "replace richard" ]] || false - [[ "$output" =~ "Johnny Moolah" ]] || false - [[ "$output" =~ "add more people" ]] || false + [[ "$output" =~ "2".*"Harry Wombat".*"replace richard" ]] || false + [[ "$output" =~ "3".*"Johnny Moolah".*"add more people" ]] || false + [[ "$output" =~ "4".*"Johnny Moolah".*"add more people" ]] || false } @test "blame: works with HEAD~1 as the commit ref" { - skip "SQL views do no support AS OF queries" - run dolt blame HEAD~1 blame_test [ "$status" -eq 0 ] + [[ "$output" =~ "Thomas Foolery" ]] || false [[ "$output" =~ "create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false @@ -110,7 +109,7 @@ SQL } @test "blame: works with HEAD~2 as the commit ref" { - skip "SQL views do no support AS OF queries" + skip "SQL views return incorrect data when using AS OF with commits that modify existing data" run dolt blame HEAD~2 blame_test [ "$status" -eq 0 ] @@ -125,10 +124,9 @@ SQL } @test "blame: works with HEAD~3 as the commit ref" { - skip "SQL views do no support AS OF queries" - run dolt blame HEAD~3 blame_test [ "$status" -eq 0 ] + [[ "$output" =~ "Thomas Foolery" ]] || false [[ "$output" =~ "create blame_test table" ]] || false [[ ! "$output" =~ "Richard Tracy" ]] || false @@ -140,10 +138,9 @@ SQL } @test "blame: returns an error when the table is not found in the given revision" { - skip "SQL views do no support AS OF queries" run dolt blame HEAD~4 blame_test - [ "$status" -eq 1 ] - [[ "$output" =~ "no table named blame_test found" ]] || false + [ "$status" -eq 0 ] + [[ "$output" = "" ]] || false } @test "blame: pk ordered output" { diff --git a/integration-tests/bats/commit.bats b/integration-tests/bats/commit.bats index 2f94b3669f..917c9bc295 100644 --- a/integration-tests/bats/commit.bats +++ b/integration-tests/bats/commit.bats @@ -51,4 +51,25 @@ teardown() { run dolt commit [ $status -eq 1 ] [[ "$output" =~ "Failed to open commit editor" ]] || false +} + +@test "commit: --skip-empty correctly skips committing when no changes are staged" { + dolt sql -q "create table t(pk int primary key);" + dolt add t + original_head=$(get_head_commit) + + # When --allow-empty and --skip-empty are both specified, the user should get an error + run dolt commit --allow-empty --skip-empty -m "commit message" + [ $status -eq 1 ] + [[ "$output" =~ 'error: cannot use both --allow-empty and --skip-empty' ]] || false + [ $original_head = $(get_head_commit) ] + + # When changes are staged, --skip-empty has no effect + dolt commit --skip-empty -m "commit message" + new_head=$(get_head_commit) + [ $original_head != $new_head ] + + # When no changes are staged, --skip-empty skips creating the commit + dolt commit --skip-empty -m "commit message" + [ $new_head = $(get_head_commit) ] } \ No newline at end of file diff --git a/integration-tests/bats/helper/common.bash b/integration-tests/bats/helper/common.bash index 7e85640e3e..c252f2ecce 100644 --- a/integration-tests/bats/helper/common.bash +++ b/integration-tests/bats/helper/common.bash @@ -38,6 +38,11 @@ current_dolt_user_email() { dolt config --global --get user.email } +# get_head_commit returns the commit hash for the current HEAD +get_head_commit() { + dolt log -n 1 | grep -m 1 commit | cut -c 13-44 +} + setup_no_dolt_init() { export PATH=$PATH:~/go/bin cd $BATS_TMPDIR diff --git a/integration-tests/bats/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~ diff --git a/integration-tests/bats/replication-multidb.bats b/integration-tests/bats/replication-multidb.bats index 7bc31d356a..04b01e4f90 100644 --- a/integration-tests/bats/replication-multidb.bats +++ b/integration-tests/bats/replication-multidb.bats @@ -52,7 +52,7 @@ teardown() { cd $BATS_TMPDIR if ! [ "$DOLT_DEFAULT_BIN_FORMAT" = "__DOLT__" ]; then - dolt config --list | awk '{ print $1 }' | grep sqlserver.global | xargs dolt config --global --unset + dolt config --list | awk '{ print $1 }' | grep sqlserver.global | xargs -r dolt config --global --unset fi } diff --git a/integration-tests/bats/sql-commit.bats b/integration-tests/bats/sql-commit.bats index 80503fd257..78eb38f871 100644 --- a/integration-tests/bats/sql-commit.bats +++ b/integration-tests/bats/sql-commit.bats @@ -453,6 +453,21 @@ SQL [[ "$output" =~ 'error: no value for option `message' ]] || false } -get_head_commit() { - dolt log -n 1 | grep -m 1 commit | cut -c 13-44 +@test "sql-commit: --skip-empty correctly skips committing when no changes are staged" { + original_head=$(get_head_commit) + + # When --allow-empty and --skip-empty are both specified, the user should get an error + run dolt sql -q "CALL DOLT_COMMIT('--allow-empty', '--skip-empty', '-m', 'commit message');" + [ $status -eq 1 ] + [[ "$output" =~ 'error: cannot use both --allow-empty and --skip-empty' ]] || false + [ $original_head = $(get_head_commit) ] + + # When changes are staged, --skip-empty has no effect + dolt sql -q "CALL DOLT_COMMIT('--skip-empty', '-m', 'commit message');" + new_head=$(get_head_commit) + [ $original_head != $new_head ] + + # When no changes are staged, --skip-empty skips creating the commit + dolt sql -q "CALL DOLT_COMMIT('--skip-empty', '-m', 'commit message');" + [ $new_head = $(get_head_commit) ] } diff --git a/integration-tests/bats/sql-load-data.bats b/integration-tests/bats/sql-load-data.bats index a0e95d43f7..045b687023 100644 --- a/integration-tests/bats/sql-load-data.bats +++ b/integration-tests/bats/sql-load-data.bats @@ -294,7 +294,6 @@ CSV dolt sql -q "create table t (pk int primary key, c1 int, c2 int)" dolt sql -q "insert into t values (0,0,0)" - skip "load data ignore not supported" run dolt sql <