From d69ac3373800645ebce143b3c738b1ce3178a0e6 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Wed, 30 Mar 2022 11:25:55 -0700 Subject: [PATCH] go/store/datas: FindCommonAncestor returns addr, not Ref. --- go/libraries/doltcore/doltdb/commit.go | 16 ++++----- go/store/cmd/noms/noms_merge.go | 6 ++-- go/store/datas/commit.go | 50 ++++++++++---------------- go/store/datas/commit_test.go | 12 +++---- go/store/datas/database_common.go | 4 +-- 5 files changed, 37 insertions(+), 51 deletions(-) diff --git a/go/libraries/doltcore/doltdb/commit.go b/go/libraries/doltcore/doltdb/commit.go index a0ccbe6234..cf4af49e49 100644 --- a/go/libraries/doltcore/doltdb/commit.go +++ b/go/libraries/doltcore/doltdb/commit.go @@ -128,14 +128,12 @@ func GetCommitAncestor(ctx context.Context, cm1, cm2 *Commit) (*Commit, error) { return nil, err } - ref, err := getCommitAncestorRef(ctx, ref1, ref2, cm1.vrw, cm2.vrw) - + addr, err := getCommitAncestorAddr(ctx, ref1, ref2, cm1.vrw, cm2.vrw) if err != nil { return nil, err } - targetVal, err := ref.TargetValue(ctx, cm1.vrw) - + targetVal, err := cm1.vrw.ReadValue(ctx, addr) if err != nil { return nil, err } @@ -143,18 +141,18 @@ func GetCommitAncestor(ctx context.Context, cm1, cm2 *Commit) (*Commit, error) { return NewCommit(ctx, cm1.vrw, targetVal) } -func getCommitAncestorRef(ctx context.Context, ref1, ref2 types.Ref, vrw1, vrw2 types.ValueReadWriter) (types.Ref, error) { - ancestorRef, ok, err := datas.FindCommonAncestor(ctx, ref1, ref2, vrw1, vrw2) +func getCommitAncestorAddr(ctx context.Context, ref1, ref2 types.Ref, vrw1, vrw2 types.ValueReadWriter) (hash.Hash, error) { + ancestorAddr, ok, err := datas.FindCommonAncestor(ctx, ref1, ref2, vrw1, vrw2) if err != nil { - return types.Ref{}, err + return hash.Hash{}, err } if !ok { - return types.Ref{}, ErrNoCommonAncestor + return hash.Hash{}, ErrNoCommonAncestor } - return ancestorRef, nil + return ancestorAddr, nil } func (c *Commit) CanFastForwardTo(ctx context.Context, new *Commit) (bool, error) { diff --git a/go/store/cmd/noms/noms_merge.go b/go/store/cmd/noms/noms_merge.go index f4b49cddc3..5271f840ef 100644 --- a/go/store/cmd/noms/noms_merge.go +++ b/go/store/cmd/noms/noms_merge.go @@ -181,15 +181,15 @@ func getMergeCandidates(ctx context.Context, db datas.Database, vrw types.ValueR } func getCommonAncestor(ctx context.Context, r1, r2 types.Ref, vr types.ValueReader) (a types.Struct, found bool) { - aRef, found, err := datas.FindCommonAncestor(ctx, r1, r2, vr, vr) + aaddr, found, err := datas.FindCommonAncestor(ctx, r1, r2, vr, vr) d.PanicIfError(err) if !found { return } - v, err := vr.ReadValue(ctx, aRef.TargetHash()) + v, err := vr.ReadValue(ctx, aaddr) d.PanicIfError(err) if v == nil { - panic(aRef.TargetHash().String() + " not found") + panic(aaddr.String() + " not found") } isCm, err := datas.IsCommit(v) diff --git a/go/store/datas/commit.go b/go/store/datas/commit.go index 3bff97593c..0767764184 100644 --- a/go/store/datas/commit.go +++ b/go/store/datas/commit.go @@ -213,37 +213,37 @@ func newCommitForValue(ctx context.Context, vrw types.ValueReadWriter, v types.V return newCommit(ctx, v, parentsList, parentsClosure, includeParentsClosure, metaSt) } -func findCommonAncestorUsingParentsList(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (types.Ref, bool, error) { +func findCommonAncestorUsingParentsList(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (hash.Hash, bool, error) { c1Q, c2Q := RefByHeightHeap{c1}, RefByHeightHeap{c2} for !c1Q.Empty() && !c2Q.Empty() { c1Ht, c2Ht := c1Q.MaxHeight(), c2Q.MaxHeight() if c1Ht == c2Ht { c1Parents, c2Parents := c1Q.PopRefsOfHeight(c1Ht), c2Q.PopRefsOfHeight(c2Ht) if common, ok := findCommonRef(c1Parents, c2Parents); ok { - return common, true, nil + return common.TargetHash(), true, nil } err := parentsToQueue(ctx, c1Parents, &c1Q, vr1) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } err = parentsToQueue(ctx, c2Parents, &c2Q, vr2) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } } else if c1Ht > c2Ht { err := parentsToQueue(ctx, c1Q.PopRefsOfHeight(c1Ht), &c1Q, vr1) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } } else { err := parentsToQueue(ctx, c2Q.PopRefsOfHeight(c2Ht), &c2Q, vr2) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } } } - return types.Ref{}, false, nil + return hash.Hash{}, false, nil } // FindCommonAncestor returns the most recent common ancestor of c1 and c2, if @@ -254,10 +254,10 @@ func findCommonAncestorUsingParentsList(ctx context.Context, c1, c2 types.Ref, v // This implementation makes use of the parents_closure field on the commit // struct. If the commit does not have a materialized parents_closure, this // implementation delegates to findCommonAncestorUsingParentsList. -func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (types.Ref, bool, error) { +func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (hash.Hash, bool, error) { pi1, err := newParentsClosureIterator(ctx, c1, vr1) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } if pi1 == nil { return findCommonAncestorUsingParentsList(ctx, c1, c2, vr1, vr2) @@ -265,7 +265,7 @@ func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.Va pi2, err := newParentsClosureIterator(ctx, c2, vr2) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } if pi2 == nil { return findCommonAncestorUsingParentsList(ctx, c1, c2, vr1, vr2) @@ -275,22 +275,18 @@ func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.Va h1, h2 := pi1.Hash(), pi2.Hash() if h1 == h2 { if err := firstError(pi1.Err(), pi2.Err()); err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } - r, err := hashToRef(ctx, vr1, h1) - if err != nil { - return types.Ref{}, false, err - } - return r, true, nil + return h1, true, nil } if pi1.Less(vr1.Format(), pi2) { // TODO: Should pi2.Seek(pi1.curr), but MapIterator does not expose Seek yet. if !pi2.Next(ctx) { - return types.Ref{}, false, firstError(pi1.Err(), pi2.Err()) + return hash.Hash{}, false, firstError(pi1.Err(), pi2.Err()) } } else { if !pi1.Next(ctx) { - return types.Ref{}, false, firstError(pi1.Err(), pi2.Err()) + return hash.Hash{}, false, firstError(pi1.Err(), pi2.Err()) } } } @@ -299,7 +295,7 @@ func FindCommonAncestor(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.Va // FindClosureCommonAncestor returns the most recent common ancestor of |cl| and |cm|, // where |cl| is the transitive closure of one or more refs. If a common ancestor // exists, |ok| is set to true, else false. -func FindClosureCommonAncestor(ctx context.Context, cl RefClosure, cm types.Ref, vr types.ValueReader) (a types.Ref, ok bool, err error) { +func FindClosureCommonAncestor(ctx context.Context, cl RefClosure, cm types.Ref, vr types.ValueReader) (a hash.Hash, ok bool, err error) { q := &RefByHeightHeap{cm} var curr types.RefSlice @@ -309,20 +305,20 @@ func FindClosureCommonAncestor(ctx context.Context, cl RefClosure, cm types.Ref, for _, r := range curr { ok, err = cl.Contains(ctx, r) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } if ok { - return r, ok, nil + return r.TargetHash(), ok, nil } } err = parentsToQueue(ctx, curr, q, vr) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } } - return types.Ref{}, false, nil + return hash.Hash{}, false, nil } // GetCommitParents returns |Ref|s to the parents of the commit. @@ -591,14 +587,6 @@ func (i *parentsClosureIterator) Next(ctx context.Context) bool { return true } -func hashToRef(ctx context.Context, vr types.ValueReader, h hash.Hash) (types.Ref, error) { - fetched, err := vr.ReadValue(ctx, h) - if err != nil { - return types.Ref{}, err - } - return types.NewRef(fetched, vr.Format()) -} - func refToMapKeyTuple(f *types.NomsBinFormat, r types.Ref) (types.Tuple, error) { h := r.TargetHash() ib := make([]byte, len(hash.Hash{})) diff --git a/go/store/datas/commit_test.go b/go/store/datas/commit_test.go index 2e620d486b..ceb8a0247e 100644 --- a/go/store/datas/commit_test.go +++ b/go/store/datas/commit_test.go @@ -289,22 +289,22 @@ func toRefList(vrw types.ValueReadWriter, commits ...types.Struct) (types.List, return le.List(context.Background()) } -func commonAncWithSetClosure(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a types.Ref, ok bool, err error) { +func commonAncWithSetClosure(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a hash.Hash, ok bool, err error) { closure, err := NewSetRefClosure(ctx, vr1, c1) if err != nil { - return types.Ref{}, false, err + return hash.Hash{}, false, err } return FindClosureCommonAncestor(ctx, closure, c2, vr2) } -func commonAncWithLazyClosure(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a types.Ref, ok bool, err error) { +func commonAncWithLazyClosure(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a hash.Hash, ok bool, err error) { closure := NewLazyRefClosure(c1, vr1) return FindClosureCommonAncestor(ctx, closure, c2, vr2) } // Assert that c is the common ancestor of a and b, using multiple common ancestor methods. func assertCommonAncestor(t *testing.T, expected, a, b types.Value, ldb, rdb *database) { - type caFinder func(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a types.Ref, ok bool, err error) + type caFinder func(ctx context.Context, c1, c2 types.Ref, vr1, vr2 types.ValueReader) (a hash.Hash, ok bool, err error) methods := map[string]caFinder{ "FindCommonAncestor": FindCommonAncestor, @@ -323,7 +323,7 @@ func assertCommonAncestor(t *testing.T, expected, a, b types.Value, ldb, rdb *da found, ok, err := method(ctx, aref, bref, ldb, rdb) assert.NoError(err) if assert.True(ok) { - tv, err := found.TargetValue(context.Background(), ldb) + tv, err := ldb.ReadValue(context.Background(), found) assert.NoError(err) ancestor := tv expV, _ := GetCommitValue(ctx, ldb, expected) @@ -594,7 +594,7 @@ func TestFindCommonAncestor(t *testing.T) { if !assert.False(ok) { d2V, _ := GetCommitValue(ctx, db, d2) a6V, _ := GetCommitValue(ctx, db, a6) - fTV, _ := found.TargetValue(ctx, db) + fTV, _ := db.ReadValue(ctx, found) fV, _ := GetCommitValue(ctx, db, fTV) assert.Fail( diff --git a/go/store/datas/database_common.go b/go/store/datas/database_common.go index a26daa624e..1ea6ea0967 100644 --- a/go/store/datas/database_common.go +++ b/go/store/datas/database_common.go @@ -490,11 +490,11 @@ func (db *database) doFastForward(ctx context.Context, ds Dataset, newHeadAddr h return err } else if ok { currentHeadAddr = ref.TargetHash() - ancestorRef, found, err := FindCommonAncestor(ctx, ref, newRef, db, db) + ancestorHash, found, err := FindCommonAncestor(ctx, ref, newRef, db, db) if err != nil { return err } - if !found || mergeNeeded(currentHeadAddr, ancestorRef.TargetHash()) { + if !found || mergeNeeded(currentHeadAddr, ancestorHash) { return ErrMergeNeeded } }