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/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/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..1c7e44355b 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.2.5" ) 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 ae2053f4fb..b133ef66ca 100644 --- a/go/go.mod +++ b/go/go.mod @@ -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.20230605224424-ada1bacfb2d3 + github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2 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 fe1ecf62f5..bcbe260f93 100644 --- a/go/go.sum +++ b/go/go.sum @@ -168,10 +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.20230602002747-63ffc9c1b72f h1:f7H66wimomDcGtzBpTa7slxUIPUKY0t3nweGuS/GgQo= -github.com/dolthub/go-mysql-server v0.15.1-0.20230602002747-63ffc9c1b72f/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= -github.com/dolthub/go-mysql-server v0.15.1-0.20230605224424-ada1bacfb2d3 h1:/CHoQMpuL+Oy2r/Zy5k39GoDYZY29kyV/+L4bi4cS/g= -github.com/dolthub/go-mysql-server v0.15.1-0.20230605224424-ada1bacfb2d3/go.mod h1:E1STXoY8HvbrOWwFOhHpMfIjgcjboBhFtSpRxPgiBOw= +github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2 h1:pbu+PFvYJ8q8GJ8O8ni6H2md7HXfhvnub9BPAiVVj5g= +github.com/dolthub/go-mysql-server v0.15.1-0.20230605201547-bf3de319a3c2/go.mod h1:uXHw3YMx/PLUe5vUaFDdr66lVHiabnn0xi1LZg3T+FA= 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= 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 9be838ed59..6c0c2986bf 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,12 +28,14 @@ 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/env" "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/ref" "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 ( @@ -544,12 +547,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_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 2e9c5fa9dd..9027c5f7b2 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4019,30 +4019,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 8759096130..ebd07816d6 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/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-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index db707e557e..b3ce627ffd 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -11,6 +11,9 @@ make_repo() { } setup() { + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "This test tests remote connections directly, SQL_ENGINE is not needed." + fi setup_no_dolt_init make_repo defaultDB make_repo altDB @@ -21,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 @@ -129,4 +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 +} \ No newline at end of file