From 3bf3cf43c8de65abd63c09c21a10e36f04335775 Mon Sep 17 00:00:00 2001 From: Andy Arthur Date: Mon, 14 Mar 2022 11:46:06 -0700 Subject: [PATCH] unified merge() and dolt_merge(), updated BATS to stage changes before committing, some sql-server tests still failing --- .../doltcore/sqle/dfunctions/commit.go | 2 +- .../doltcore/sqle/dfunctions/dolt_commit.go | 4 +- .../doltcore/sqle/dfunctions/dolt_merge.go | 33 +++- .../doltcore/sqle/dfunctions/merge.go | 187 +----------------- .../doltcore/sqle/dfunctions/squash.go | 50 +++++ integration-tests/bats/config.bats | 12 +- integration-tests/bats/replication.bats | 1 + .../bats/server_multiclient_test.py | 2 +- integration-tests/bats/sql-server.bats | 22 +-- 9 files changed, 99 insertions(+), 214 deletions(-) diff --git a/go/libraries/doltcore/sqle/dfunctions/commit.go b/go/libraries/doltcore/sqle/dfunctions/commit.go index 955c717630..7ec050a773 100644 --- a/go/libraries/doltcore/sqle/dfunctions/commit.go +++ b/go/libraries/doltcore/sqle/dfunctions/commit.go @@ -34,7 +34,7 @@ func NewCommitFunc(args ...sql.Expression) (sql.Expression, error) { // Eval implements the Expression interface. func (cf *CommitFunc) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { - return makeDoltCommit(ctx, row, cf.Children()) + return doDoltCommit(ctx, row, cf.Children()) } // String implements the Stringer interface. diff --git a/go/libraries/doltcore/sqle/dfunctions/dolt_commit.go b/go/libraries/doltcore/sqle/dfunctions/dolt_commit.go index 3f4b76f4a1..746d997ac7 100644 --- a/go/libraries/doltcore/sqle/dfunctions/dolt_commit.go +++ b/go/libraries/doltcore/sqle/dfunctions/dolt_commit.go @@ -41,10 +41,10 @@ func NewDoltCommitFunc(args ...sql.Expression) (sql.Expression, error) { } func (d DoltCommitFunc) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { - return makeDoltCommit(ctx, row, d.Children()) + return doDoltCommit(ctx, row, d.Children()) } -func makeDoltCommit(ctx *sql.Context, row sql.Row, exprs []sql.Expression) (interface{}, error) { +func doDoltCommit(ctx *sql.Context, row sql.Row, exprs []sql.Expression) (interface{}, error) { // Get the information for the sql context. dbName := ctx.GetCurrentDatabase() ap := cli.CreateCommitArgParser() diff --git a/go/libraries/doltcore/sqle/dfunctions/dolt_merge.go b/go/libraries/doltcore/sqle/dfunctions/dolt_merge.go index be983572ed..e21f44a51f 100644 --- a/go/libraries/doltcore/sqle/dfunctions/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dfunctions/dolt_merge.go @@ -32,6 +32,10 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/argparser" ) +func NewDoltMergeFunc(args ...sql.Expression) (sql.Expression, error) { + return &DoltMergeFunc{expression.NaryExpression{ChildExpressions: args}}, nil +} + const DoltMergeFuncName = "dolt_merge" type DoltMergeFunc struct { @@ -46,6 +50,10 @@ const ( ) func (d DoltMergeFunc) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { + return doDoltCommit(ctx, row, d.Children()) +} + +func doDoltMerge(ctx *sql.Context, row sql.Row, exprs []sql.Expression) (interface{}, error) { dbName := ctx.GetCurrentDatabase() if len(dbName) == 0 { @@ -55,7 +63,7 @@ func (d DoltMergeFunc) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) sess := dsess.DSessFromSess(ctx.Session) ap := cli.CreateMergeArgParser() - args, err := getDoltArgs(ctx, row, d.Children()) + args, err := getDoltArgs(ctx, row, exprs) if err != nil { return noConflicts, err @@ -401,6 +409,25 @@ func mergeRootToWorking( return ws, nil } +func checkForUncommittedChanges(root *doltdb.RootValue, headRoot *doltdb.RootValue) error { + rh, err := root.HashOf() + + if err != nil { + return err + } + + hrh, err := headRoot.HashOf() + + if err != nil { + return err + } + + if rh != hrh { + return ErrUncommittedChanges.New() + } + return nil +} + func checkForConflicts(tblToStats map[string]*merge.MergeStats) bool { for _, stats := range tblToStats { if stats.Operation == merge.TableModified && stats.Conflicts > 0 { @@ -428,7 +455,3 @@ func (d DoltMergeFunc) Type() sql.Type { func (d DoltMergeFunc) WithChildren(children ...sql.Expression) (sql.Expression, error) { return NewDoltMergeFunc(children...) } - -func NewDoltMergeFunc(args ...sql.Expression) (sql.Expression, error) { - return &DoltMergeFunc{expression.NaryExpression{ChildExpressions: args}}, nil -} diff --git a/go/libraries/doltcore/sqle/dfunctions/merge.go b/go/libraries/doltcore/sqle/dfunctions/merge.go index f084500e7c..4bcfe3ac63 100644 --- a/go/libraries/doltcore/sqle/dfunctions/merge.go +++ b/go/libraries/doltcore/sqle/dfunctions/merge.go @@ -15,19 +15,11 @@ package dfunctions import ( - "errors" "fmt" "strings" "github.com/dolthub/go-mysql-server/sql" goerrors "gopkg.in/src-d/go-errors.v1" - - "github.com/dolthub/dolt/go/cmd/dolt/cli" - "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" - "github.com/dolthub/dolt/go/libraries/doltcore/merge" - "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" - "github.com/dolthub/dolt/go/store/datas" - "github.com/dolthub/dolt/go/store/hash" ) const MergeFuncName = "merge" @@ -46,184 +38,7 @@ func NewMergeFunc(args ...sql.Expression) (sql.Expression, error) { // Eval implements the Expression interface. // todo(andy): merge with DOLT_MERGE() func (cf *MergeFunc) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { - sess := dsess.DSessFromSess(ctx.Session) - - // TODO: Move to a separate MERGE argparser. - ap := cli.CreateCommitArgParser() - args, err := getDoltArgs(ctx, row, cf.Children()) - - if err != nil { - return nil, err - } - - apr, err := ap.Parse(args) - if err != nil { - return nil, err - } - - // The fist argument should be the branch name. - branchName := apr.Arg(0) - - var name, email string - if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { - name, email, err = cli.ParseAuthor(authorStr) - if err != nil { - return nil, err - } - } else { - name = sess.Username() - email = sess.Email() - } - - dbName := sess.GetCurrentDatabase() - ddb, ok := sess.GetDoltDB(ctx, dbName) - if !ok { - return nil, sql.ErrDatabaseNotFound.New(dbName) - } - - roots, ok := sess.GetRoots(ctx, dbName) - if !ok { - return nil, sql.ErrDatabaseNotFound.New(dbName) - } - - head, hh, headRoot, err := getHead(ctx, sess, dbName) - if err != nil { - return nil, err - } - - err = checkForUncommittedChanges(roots.Working, headRoot) - if err != nil { - return nil, err - } - - cm, cmh, err := getBranchCommit(ctx, branchName, ddb) - if err != nil { - return nil, err - } - - // No need to write a merge commit, if the head can ffw to the commit coming from the branch. - canFF, err := head.CanFastForwardTo(ctx, cm) - if err != nil { - return nil, err - } - - if canFF { - ancRoot, err := head.GetRootValue() - if err != nil { - return nil, err - } - mergedRoot, err := cm.GetRootValue() - if err != nil { - return nil, err - } - if cvPossible, err := merge.MayHaveConstraintViolations(ctx, ancRoot, mergedRoot); err != nil { - return nil, err - } else if !cvPossible { - return cmh.String(), nil - } - } - - dbState, ok, err := sess.LookupDbState(ctx, dbName) - if err != nil { - return nil, err - } else if !ok { - return nil, fmt.Errorf("Could not load database %s", dbName) - } - - mergeRoot, _, err := merge.MergeCommits(ctx, head, cm, dbState.EditOpts()) - - if err != nil { - return nil, err - } - - h, err := ddb.WriteRootValue(ctx, mergeRoot) - if err != nil { - return nil, err - } - - commitMessage := fmt.Sprintf("SQL Generated commit merging %s into %s", hh.String(), cmh.String()) - meta, err := datas.NewCommitMeta(name, email, commitMessage) - if err != nil { - return nil, err - } - - mergeCommit, err := ddb.CommitDanglingWithParentCommits(ctx, h, []*doltdb.Commit{head, cm}, meta) - if err != nil { - return nil, err - } - - h, err = mergeCommit.HashOf() - if err != nil { - return nil, err - } - - return h.String(), nil -} - -func checkForUncommittedChanges(root *doltdb.RootValue, headRoot *doltdb.RootValue) error { - rh, err := root.HashOf() - - if err != nil { - return err - } - - hrh, err := headRoot.HashOf() - - if err != nil { - return err - } - - if rh != hrh { - return ErrUncommittedChanges.New() - } - return nil -} - -func getBranchCommit(ctx *sql.Context, val interface{}, ddb *doltdb.DoltDB) (*doltdb.Commit, hash.Hash, error) { - paramStr, ok := val.(string) - - if !ok { - return nil, hash.Hash{}, errors.New("branch name is not a string") - } - - branchRef, err := getBranchInsensitive(ctx, paramStr, ddb) - - if err != nil { - return nil, hash.Hash{}, err - } - - cm, err := ddb.ResolveCommitRef(ctx, branchRef) - - if err != nil { - return nil, hash.Hash{}, err - } - - cmh, err := cm.HashOf() - - if err != nil { - return nil, hash.Hash{}, err - } - - return cm, cmh, nil -} - -func getHead(ctx *sql.Context, sess *dsess.DoltSession, dbName string) (*doltdb.Commit, hash.Hash, *doltdb.RootValue, error) { - head, err := sess.GetHeadCommit(ctx, dbName) - if err != nil { - return nil, hash.Hash{}, nil, err - } - - hh, err := head.HashOf() - if err != nil { - return nil, hash.Hash{}, nil, err - } - - headRoot, err := head.GetRootValue() - if err != nil { - return nil, hash.Hash{}, nil, err - } - - return head, hh, headRoot, nil + return doDoltMerge(ctx, row, cf.Children()) } // String implements the Stringer interface. diff --git a/go/libraries/doltcore/sqle/dfunctions/squash.go b/go/libraries/doltcore/sqle/dfunctions/squash.go index be1a4a0034..ab5364c14e 100644 --- a/go/libraries/doltcore/sqle/dfunctions/squash.go +++ b/go/libraries/doltcore/sqle/dfunctions/squash.go @@ -15,13 +15,16 @@ package dfunctions import ( + "errors" "fmt" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/expression" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" + "github.com/dolthub/dolt/go/store/hash" ) const SquashFuncName = "squash" @@ -120,3 +123,50 @@ func (s SquashFunc) WithChildren(children ...sql.Expression) (sql.Expression, er return NewSquashFunc(children[0]), nil } + +func getBranchCommit(ctx *sql.Context, val interface{}, ddb *doltdb.DoltDB) (*doltdb.Commit, hash.Hash, error) { + paramStr, ok := val.(string) + + if !ok { + return nil, hash.Hash{}, errors.New("branch name is not a string") + } + + branchRef, err := getBranchInsensitive(ctx, paramStr, ddb) + + if err != nil { + return nil, hash.Hash{}, err + } + + cm, err := ddb.ResolveCommitRef(ctx, branchRef) + + if err != nil { + return nil, hash.Hash{}, err + } + + cmh, err := cm.HashOf() + + if err != nil { + return nil, hash.Hash{}, err + } + + return cm, cmh, nil +} + +func getHead(ctx *sql.Context, sess *dsess.DoltSession, dbName string) (*doltdb.Commit, hash.Hash, *doltdb.RootValue, error) { + head, err := sess.GetHeadCommit(ctx, dbName) + if err != nil { + return nil, hash.Hash{}, nil, err + } + + hh, err := head.HashOf() + if err != nil { + return nil, hash.Hash{}, nil, err + } + + headRoot, err := head.GetRootValue() + if err != nil { + return nil, hash.Hash{}, nil, err + } + + return head, hh, headRoot, nil +} diff --git a/integration-tests/bats/config.bats b/integration-tests/bats/config.bats index 3d381ca194..24d5912113 100644 --- a/integration-tests/bats/config.bats +++ b/integration-tests/bats/config.bats @@ -208,20 +208,16 @@ teardown() { dolt config --global --add user.email "joshn@doe.com" dolt init - dolt sql -q " - CREATE TABLE test ( - pk int primary key - )" + dolt sql -q "CREATE TABLE test (pk int primary key)" dolt config --global --unset user.name dolt config --global --unset user.email - run dolt sql -q "SET @@dolt_repo_$$_head = COMMIT('-a', '-m', 'updated stuff')" - [ "$status" -eq 0 ] + dolt sql -q "SET @@dolt_repo_$$_head = COMMIT('-a', '-m', 'updated stuff')" dolt config --global --add user.name "bats tester" - run dolt sql -q "SET @@dolt_repo_$$_head = COMMIT('-a', '-m', 'updated stuff')" - [ "$status" -eq 0 ] + dolt sql -q "INSERT INTO test VALUES (1);" + dolt sql -q "SET @@dolt_repo_$$_head = COMMIT('-a', '-m', 'updated stuff')" } @test "config: DOLT_COMMIT uses default values when user.name or user.email is unset." { diff --git a/integration-tests/bats/replication.bats b/integration-tests/bats/replication.bats index 146330a1d6..dfcccf8f59 100644 --- a/integration-tests/bats/replication.bats +++ b/integration-tests/bats/replication.bats @@ -90,6 +90,7 @@ teardown() { cd repo1 dolt config --local --add sqlserver.global.dolt_replicate_to_remote backup1 dolt sql -q "create table t1 (a int primary key)" + dolt add -A dolt sql -q "UPDATE dolt_branches SET hash = COMMIT('--author', '{user_name} <{email_address}>','-m', 'cm') WHERE name = 'main' AND hash = @@repo1_head" cd .. diff --git a/integration-tests/bats/server_multiclient_test.py b/integration-tests/bats/server_multiclient_test.py index b0b60e7951..5d07a808ef 100644 --- a/integration-tests/bats/server_multiclient_test.py +++ b/integration-tests/bats/server_multiclient_test.py @@ -35,7 +35,7 @@ def commit_and_update_branch(dc, commit_message, expected_hashes, branch_name): expected_hash += "hash = %s" % eh expected_hash += ")" - query_str = 'UPDATE dolt_branches SET hash = Commit("-m", "%s") WHERE name = "%s" AND %s' % (commit_message, branch_name, expected_hash) + query_str = 'UPDATE dolt_branches SET hash = Commit("-am", "%s") WHERE name = "%s" AND %s' % (commit_message, branch_name, expected_hash) _, row_count = query(dc, query_str) if row_count != 1: diff --git a/integration-tests/bats/sql-server.bats b/integration-tests/bats/sql-server.bats index a9c86875d4..e115d3dfc4 100644 --- a/integration-tests/bats/sql-server.bats +++ b/integration-tests/bats/sql-server.bats @@ -303,7 +303,7 @@ teardown() { INSERT INTO one_pk (pk) VALUES (0); INSERT INTO one_pk (pk,c1) VALUES (1,1); INSERT INTO one_pk (pk,c1,c2) VALUES (2,2,2),(3,3,3); - SET @@repo1_head=commit('-m', 'test commit message', '--author', 'John Doe '); + SET @@repo1_head=commit('-am', 'test commit message', '--author', 'John Doe '); INSERT INTO dolt_branches (name,hash) VALUES ('test_branch', @@repo1_head);" # validate new branch was created @@ -347,7 +347,7 @@ teardown() { INSERT INTO one_pk (pk) VALUES (0); INSERT INTO one_pk (pk,c1) VALUES (1,1); INSERT INTO one_pk (pk,c1,c2) VALUES (2,2,2),(3,3,3); - SET @@repo1_head=commit('-m', 'test commit message'); + SET @@repo1_head=commit('-am', 'test commit message'); INSERT INTO dolt_branches (name,hash) VALUES ('test_branch', @@repo1_head);" # validate new branch was created @@ -372,12 +372,12 @@ teardown() { multi_query repo1 0 " SET @@repo1_head=hashof('main'); UPDATE one_pk SET c1=10 WHERE pk=2; - SET @@repo1_head=commit('-m', 'Change c 1 to 10'); + SET @@repo1_head=commit('-am', 'Change c 1 to 10'); INSERT INTO dolt_branches (name,hash) VALUES ('main', @@repo1_head); SET @@repo1_head=hashof('test_branch'); INSERT INTO one_pk (pk,c1,c2) VALUES (4,4,4); - SET @@repo1_head=commit('-m', 'add 4'); + SET @@repo1_head=commit('-am', 'add 4'); INSERT INTO dolt_branches (name,hash) VALUES ('test_branch', @@repo1_head);" multi_query repo1 0 " @@ -421,10 +421,10 @@ teardown() { INSERT INTO one_pk (pk) VALUES (0); INSERT INTO one_pk (pk,c1) VALUES (1,1); INSERT INTO one_pk (pk,c1,c2) VALUES (2,2,2),(3,3,3); - SET @@repo1_head=commit('-m', 'test commit message'); + SET @@repo1_head=commit('-am', 'test commit message'); INSERT INTO dolt_branches (name,hash) VALUES ('test_branch', @@repo1_head); INSERT INTO one_pk (pk,c1,c2) VALUES (4,4,4),(5,5,5); - SET @@repo1_head=commit('-m', 'second commit'); + SET @@repo1_head=commit('-am', 'second commit'); INSERT INTO dolt_branches (name,hash) VALUES ('test_branch', @@repo1_head); " @@ -437,7 +437,7 @@ teardown() { # Squash the test_branch into main even though it is a fast-forward merge. multi_query repo1 0 " SET @@repo1_working = squash('test_branch'); - SET @@repo1_head = COMMIT('-m', 'cm1'); + SET @@repo1_head = COMMIT('-am', 'cm1'); UPDATE dolt_branches SET hash = @@repo1_head WHERE name= 'main';" # Validate tables and data on main @@ -451,14 +451,14 @@ teardown() { multi_query repo1 0 " SET @@repo1_head=hashof('main'); UPDATE one_pk SET c1=10 WHERE pk=2; - SET @@repo1_head=commit('-m', 'Change c 1 to 10'); + SET @@repo1_head=commit('-am', 'Change c 1 to 10'); UPDATE dolt_branches SET hash = @@repo1_head WHERE name= 'main'; SET @@repo1_head=hashof('test_branch'); INSERT INTO one_pk (pk,c1,c2) VALUES (6,6,6); - SET @@repo1_head=commit('-m', 'add 6'); + SET @@repo1_head=commit('-am', 'add 6'); INSERT INTO one_pk (pk,c1,c2) VALUES (7,7,7); - SET @@repo1_head=commit('-m', 'add 7'); + SET @@repo1_head=commit('-am', 'add 7'); INSERT INTO dolt_branches (name,hash) VALUES ('test_branch', @@repo1_head);" # Validate that running a squash operation without updating the working variable itself alone does not @@ -871,7 +871,7 @@ SQL PRIMARY KEY (pk) ); INSERT INTO one_pk (pk,c1,c2) VALUES (2,2,2),(3,3,3); - SET @@repo1_head=commit('-m', 'test commit message', '--author', 'John Doe '); + SET @@repo1_head=commit('-am', 'test commit message', '--author', 'John Doe '); INSERT INTO dolt_branches (name,hash) VALUES ('main', @@repo1_head);" dolt add .