go/store/datas: GetCommitParents: Make parent walks work on commit structs/messages, not Refs. Step towards removing heights from message refs.

2026-03-06 00:58:43 -06:00 · 2022-03-30 14:52:15 -07:00
parent be27368901
commit 8f4cb102db
8 changed files with 237 additions and 140 deletions
--- a/go/libraries/doltcore/doltdb/commit.go
+++ b/go/libraries/doltcore/doltdb/commit.go
@@ -31,7 +31,7 @@ var errHasNoRootValue = errors.New("no root value")
 type Commit struct {
 	vrw     types.ValueReadWriter
 	meta    *datas.CommitMeta
-	parents []types.Ref
+	parents []*datas.Commit
 	stref   types.Ref
 	rootV   types.Value
 }
@@ -69,8 +69,8 @@ func (c *Commit) GetCommitMeta() (*datas.CommitMeta, error) {
 	return c.meta, nil
 }

-// ParentRefs returns the noms types.Refs for the commits
-func (c *Commit) ParentRefs() []types.Ref {
+// DatasParents returns the []*datas.Commit of the commit parents.
+func (c *Commit) DatasParents() []*datas.Commit {
 	return c.parents
 }

@@ -78,7 +78,7 @@ func (c *Commit) ParentRefs() []types.Ref {
 func (c *Commit) ParentHashes(ctx context.Context) ([]hash.Hash, error) {
 	hashes := make([]hash.Hash, len(c.parents))
 	for i, pr := range c.parents {
-		hashes[i] = pr.TargetHash()
+		hashes[i] = pr.Addr()
 	}
 	return hashes, nil
 }
@@ -107,11 +107,8 @@ func (c *Commit) GetStRef() (types.Ref, error) {
 }

 func (c *Commit) GetParent(ctx context.Context, idx int) (*Commit, error) {
-	ref := c.parents[idx]
-	v, err := ref.TargetValue(ctx, c.vrw)
-	if err != nil {
-		return nil, err
-	}
+	p := c.parents[idx]
+	v := p.NomsValue()
 	return NewCommit(ctx, c.vrw, v)
 }

--- a/go/libraries/doltcore/merge/revert.go
+++ b/go/libraries/doltcore/merge/revert.go
@@ -51,7 +51,7 @@ func Revert(ctx context.Context, ddb *doltdb.DoltDB, root *doltdb.RootValue, com
 		revertMessage = fmt.Sprintf(`%s "%s"`, revertMessage, baseMeta.Description)

 		var theirRoot *doltdb.RootValue
-		if len(baseCommit.ParentRefs()) > 0 {
+		if len(baseCommit.DatasParents()) > 0 {
 			parentCM, err := ddb.ResolveParent(ctx, baseCommit, 0)
 			if err != nil {
 				return nil, "", err
--- a/go/store/cmd/noms/commit_iterator.go
+++ b/go/store/cmd/noms/commit_iterator.go
@@ -28,6 +28,7 @@ import (
 	"github.com/dolthub/dolt/go/store/d"

 	"github.com/dolthub/dolt/go/store/datas"
+	"github.com/dolthub/dolt/go/store/hash"
 	"github.com/dolthub/dolt/go/store/types"
 )

@@ -40,8 +41,7 @@ type CommitIterator struct {
 func NewCommitIterator(vr types.ValueReader, commit types.Struct) *CommitIterator {
 	cr, err := types.NewRef(commit, vr.Format())
 	d.PanicIfError(err)
-
-	return &CommitIterator{vr: vr, branches: branchList{branch{cr: cr, commit: commit}}}
+	return &CommitIterator{vr: vr, branches: branchList{branch{addr: cr.TargetHash(), height: cr.Height(), commit: commit}}}
 }

 // Next returns information about the next commit to be printed. LogNode contains enough contextual
@@ -71,20 +71,18 @@ func (iter *CommitIterator) Next(ctx context.Context) (LogNode, bool) {
 	// and splice that into the iterators list of branches.
 	branches := branchList{}

-	refs, err := datas.GetCommitParents(ctx, iter.vr, br.commit)
+	parents, err := datas.GetCommitParents(ctx, iter.vr, br.commit)
 	d.PanicIfError(err)
-	for _, r := range refs {
-		v, err := iter.vr.ReadValue(ctx, r.TargetHash())
-		d.PanicIfError(err)
-
-		b := branch{cr: r, commit: v.(types.Struct)}
+	for _, p := range parents {
+		v := p.NomsValue()
+		b := branch{height: p.Height(), addr: p.Addr(), commit: v.(types.Struct)}
 		branches = append(branches, b)
 	}
 	iter.branches = iter.branches.Splice(col, 1, branches...)

 	// Collect the indexes for any newly created branches.
 	newCols := []int{}
-	for cnt := 1; cnt < len(refs); cnt++ {
+	for cnt := 1; cnt < len(parents); cnt++ {
 		newCols = append(newCols, col+cnt)
 	}

@@ -92,7 +90,8 @@ func (iter *CommitIterator) Next(ctx context.Context) (LogNode, bool) {
 	// ancestors that will be folded together on this commit's graph.
 	foldedCols := iter.branches.HighestBranchIndexes()
 	node := LogNode{
-		cr:               br.cr,
+		height:           br.height,
+		addr:             br.addr,
 		commit:           br.commit,
 		startingColCount: startingColCount,
 		endingColCount:   len(iter.branches),
@@ -105,7 +104,8 @@ func (iter *CommitIterator) Next(ctx context.Context) (LogNode, bool) {
 }

 type LogNode struct {
-	cr               types.Ref    // typed ref of commit to be printed
+	addr             hash.Hash
+	height           uint64
 	commit           types.Struct // commit that needs to be printed
 	startingColCount int          // how many branches are being tracked when this commit is printed
 	endingColCount   int          // home many branches will be tracked when next commit is printed
@@ -116,7 +116,7 @@ type LogNode struct {
 }

 func (n LogNode) String() string {
-	return fmt.Sprintf("cr: %s(%d), startingColCount: %d, endingColCount: %d, col: %d, newCols: %v, foldedCols: %v, expanding: %t, shrunk: %t, shrinking: %t", n.cr.TargetHash().String()[0:9], n.cr.Height(), n.startingColCount, n.endingColCount, n.col, n.newCols, n.foldedCols, n.Expanding(), n.Shrunk(), n.Shrinking())
+	return fmt.Sprintf("cr: %s(%d), startingColCount: %d, endingColCount: %d, col: %d, newCols: %v, foldedCols: %v, expanding: %t, shrunk: %t, shrinking: %t", n.addr.String()[0:9], n.height, n.startingColCount, n.endingColCount, n.col, n.newCols, n.foldedCols, n.Expanding(), n.Shrunk(), n.Shrinking())
 }

 // Expanding reports whether this commit's graph will expand to show an additional branch
@@ -135,12 +135,13 @@ func (n LogNode) Shrunk() bool {
 }

 type branch struct {
-	cr     types.Ref
+	addr   hash.Hash
+	height uint64
 	commit types.Struct
 }

 func (b branch) String() string {
-	return fmt.Sprintf("%s(%d)", b.cr.TargetHash().String()[0:9], b.cr.Height())
+	return fmt.Sprintf("%s(%d)", b.addr.String()[0:9], b.height)
 }

 type branchList []branch
@@ -155,14 +156,14 @@ func (bl branchList) IsEmpty() bool {
 // This indicates that two or more branches or converging.
 func (bl branchList) HighestBranchIndexes() []int {
 	maxHeight := uint64(0)
-	var cr types.Ref
+	var br branch
 	cols := []int{}
 	for i, b := range bl {
-		if b.cr.Height() > maxHeight {
-			maxHeight = b.cr.Height()
-			cr = b.cr
+		if b.height > maxHeight {
+			maxHeight = b.height
+			br = b
 			cols = []int{i}
-		} else if b.cr.Height() == maxHeight && b.cr.Equals(cr) {
+		} else if b.height == maxHeight && b.addr == br.addr {
 			cols = append(cols, i)
 		}
 	}
--- a/go/store/cmd/noms/noms_log.go
+++ b/go/store/cmd/noms/noms_log.go
@@ -186,12 +186,12 @@ func printCommit(ctx context.Context, node LogNode, path types.Path, w io.Writer
 	if len(parents) > 1 {
 		pstrings := make([]string, len(parents))
 		for i, p := range parents {
-			pstrings[i] = p.TargetHash().String()
+			pstrings[i] = p.Addr().String()
 		}
 		parentLabel = "Merge"
 		parentValue = strings.Join(pstrings, " ")
 	} else if len(parents) == 1 {
-		parentValue = parents[0].TargetHash().String()
+		parentValue = parents[0].Addr().String()
 	}

 	if oneline {
@@ -349,9 +349,8 @@ func writeDiffLines(ctx context.Context, node LogNode, path types.Path, vr types

 	parent := parents[0]

-	val, err := parent.TargetValue(ctx, vr)
+	val := parent.NomsValue()
 	parentCommit := val.(types.Struct)
-	d.PanicIfError(err)

 	var old, neu types.Value
 	err = functions.All(
--- a/go/store/cmd/noms/noms_merge_test.go
+++ b/go/store/cmd/noms/noms_merge_test.go
@@ -135,7 +135,7 @@ func (s *nomsMergeTestSuite) validateDataset(name string, expected types.Struct,
 		s.NoError(err)
 		s.Equal(len(vparents), len(parents), "parents were not the same length")
 		for i := range parents {
-			s.True(parents[i] == vparents[i].TargetHash())
+			s.True(parents[i] == vparents[i].Addr())
 		}
 		merged := mustHeadValue(sp.GetDataset(context.Background()))
 		s.True(expected.Equals(merged), "%s != %s", mustString(types.EncodedValue(context.Background(), expected)), mustString(types.EncodedValue(context.Background(), merged)))
--- a/go/store/datas/commit.go
+++ b/go/store/datas/commit.go
@@ -58,6 +58,24 @@ const (
 	commitName          = "Commit"
 )

+type Commit struct {
+	val    types.Value
+	addr   hash.Hash
+	height uint64
+}
+
+func (c *Commit) NomsValue() types.Value {
+	return c.val
+}
+
+func (c *Commit) Height() uint64 {
+	return c.height
+}
+
+func (c *Commit) Addr() hash.Hash {
+	return c.addr
+}
+
 var commitTemplateWithParentsClosure = types.MakeStructTemplate(commitName, []string{
 	commitMetaField,
 	parentsField,
@@ -213,14 +231,48 @@ 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) (hash.Hash, bool, error) {
-	c1Q, c2Q := RefByHeightHeap{c1}, RefByHeightHeap{c2}
+func commitPtr(nbf *types.NomsBinFormat, v types.Value, r types.Ref) *Commit {
+	if nbf == types.Format_DOLT_DEV {
+		height := serial.GetRootAsCommit([]byte(v.(types.SerialMessage)), 0).Height()
+		return &Commit{
+			val:    v,
+			height: height,
+			addr:   r.TargetHash(),
+		}
+	}
+	return &Commit{
+		val:    v,
+		height: r.Height(),
+		addr:   r.TargetHash(),
+	}
+}
+
+func commitPtrFromRef(ctx context.Context, vr types.ValueReader, r types.Ref) (*Commit, error) {
+	v, err := vr.ReadValue(ctx, r.TargetHash())
+	if err != nil {
+		return nil, err
+	}
+	return commitPtr(vr.Format(), v, r), nil
+}
+
+func findCommonAncestorUsingParentsList(ctx context.Context, c1r, c2r types.Ref, vr1, vr2 types.ValueReader) (hash.Hash, bool, error) {
+	c1, err := commitPtrFromRef(ctx, vr1, c1r)
+	if err != nil {
+		return hash.Hash{}, false, err
+	}
+
+	c2, err := commitPtrFromRef(ctx, vr2, c2r)
+	if err != nil {
+		return hash.Hash{}, false, err
+	}
+
+	c1Q, c2Q := CommitByHeightHeap{c1}, CommitByHeightHeap{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.TargetHash(), true, nil
+			c1Parents, c2Parents := c1Q.PopCommitsOfHeight(c1Ht), c2Q.PopCommitsOfHeight(c2Ht)
+			if common, ok := findCommonCommit(c1Parents, c2Parents); ok {
+				return common.Addr(), true, nil
 			}
 			err := parentsToQueue(ctx, c1Parents, &c1Q, vr1)
 			if err != nil {
@@ -231,12 +283,12 @@ func findCommonAncestorUsingParentsList(ctx context.Context, c1, c2 types.Ref, v
 				return hash.Hash{}, false, err
 			}
 		} else if c1Ht > c2Ht {
-			err := parentsToQueue(ctx, c1Q.PopRefsOfHeight(c1Ht), &c1Q, vr1)
+			err := parentsToQueue(ctx, c1Q.PopCommitsOfHeight(c1Ht), &c1Q, vr1)
 			if err != nil {
 				return hash.Hash{}, false, err
 			}
 		} else {
-			err := parentsToQueue(ctx, c2Q.PopRefsOfHeight(c2Ht), &c2Q, vr2)
+			err := parentsToQueue(ctx, c2Q.PopCommitsOfHeight(c2Ht), &c2Q, vr2)
 			if err != nil {
 				return hash.Hash{}, false, err
 			}
@@ -295,12 +347,17 @@ 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 hash.Hash, ok bool, err error) {
-	q := &RefByHeightHeap{cm}
-	var curr types.RefSlice
+func FindClosureCommonAncestor(ctx context.Context, cl CommitClosure, cr types.Ref, vr types.ValueReader) (a hash.Hash, ok bool, err error) {
+	cm, err := commitPtrFromRef(ctx, vr, cr)
+	if err != nil {
+		return hash.Hash{}, false, err
+	}
+
+	q := &CommitByHeightHeap{cm}
+	var curr []*Commit

 	for !q.Empty() {
-		curr = q.PopRefsOfHeight(q.MaxHeight())
+		curr = q.PopCommitsOfHeight(q.MaxHeight())

 		for _, r := range curr {
 			ok, err = cl.Contains(ctx, r)
@@ -308,7 +365,7 @@ func FindClosureCommonAncestor(ctx context.Context, cl RefClosure, cm types.Ref,
 				return hash.Hash{}, false, err
 			}
 			if ok {
-				return r.TargetHash(), ok, nil
+				return r.Addr(), ok, nil
 			}
 		}

@@ -322,13 +379,37 @@ func FindClosureCommonAncestor(ctx context.Context, cl RefClosure, cm types.Ref,
 }

 // GetCommitParents returns |Ref|s to the parents of the commit.
-func GetCommitParents(ctx context.Context, vr types.ValueReader, cv types.Value) ([]types.Ref, error) {
+func GetCommitParents(ctx context.Context, vr types.ValueReader, cv types.Value) ([]*Commit, error) {
 	if sm, ok := cv.(types.SerialMessage); ok {
 		data := []byte(sm)
 		if serial.GetFileID(data) != serial.CommitFileID {
 			return nil, errors.New("GetCommitParents: provided value is not a commit.")
 		}
-		return types.SerialCommitParentRefs(vr.Format(), sm)
+		refs, err := types.SerialCommitParentRefs(vr.Format(), sm)
+		if err != nil {
+			return nil, err
+		}
+		hashes := make([]hash.Hash, len(refs))
+		for i, r := range refs {
+			hashes[i] = r.TargetHash()
+		}
+		vals, err := vr.ReadManyValues(ctx, hashes)
+		if err != nil {
+			return nil, err
+		}
+		res := make([]*Commit, len(vals))
+		for i, v := range vals {
+			if v == nil {
+				return nil, fmt.Errorf("GetCommitParents: Did not find parent Commit in ValueReader: %s", hashes[i].String())
+			}
+			csm := serial.GetRootAsCommit([]byte(v.(types.SerialMessage)), 0)
+			res[i] = &Commit{
+				val:    v,
+				height: csm.Height(),
+				addr:   hashes[i],
+			}
+		}
+		return res, nil
 	}
 	c, ok := cv.(types.Struct)
 	if !ok {
@@ -337,7 +418,7 @@ func GetCommitParents(ctx context.Context, vr types.ValueReader, cv types.Value)
 	if c.Name() != commitName {
 		return nil, errors.New("GetCommitParents: provided value is not a commit.")
 	}
-	var ret []types.Ref
+	var refs []types.Ref
 	ps, ok, err := c.MaybeGet(parentsListField)
 	if err != nil {
 		return nil, err
@@ -345,24 +426,42 @@ func GetCommitParents(ctx context.Context, vr types.ValueReader, cv types.Value)
 	if ok {
 		p := ps.(types.List)
 		err = p.IterAll(ctx, func(v types.Value, _ uint64) error {
-			ret = append(ret, v.(types.Ref))
+			refs = append(refs, v.(types.Ref))
 			return nil
 		})
-		return ret, err
+	} else {
+		ps, ok, err = c.MaybeGet(parentsField)
+		if err != nil {
+			return nil, err
+		}
+		if ok {
+			p := ps.(types.Set)
+			err = p.IterAll(ctx, func(v types.Value) error {
+				refs = append(refs, v.(types.Ref))
+				return nil
+			})
+		}
 	}
-	ps, ok, err = c.MaybeGet(parentsField)
+	hashes := make([]hash.Hash, len(refs))
+	for i, r := range refs {
+		hashes[i] = r.TargetHash()
+	}
+	vals, err := vr.ReadManyValues(ctx, hashes)
 	if err != nil {
 		return nil, err
 	}
-	if ok {
-		p := ps.(types.Set)
-		err = p.IterAll(ctx, func(v types.Value) error {
-			ret = append(ret, v.(types.Ref))
-			return nil
-		})
-		return ret, err
+	res := make([]*Commit, len(refs))
+	for i, val := range vals {
+		if val == nil {
+			return nil, fmt.Errorf("GetCommitParents: Did not find parent Commit in ValueReader: %s", hashes[i].String())
+		}
+		res[i] = &Commit{
+			val:    val,
+			height: refs[i].Height(),
+			addr:   refs[i].TargetHash(),
+		}
 	}
-	return ret, nil
+	return res, nil
 }

 // GetCommitMeta extracts the CommitMeta field from a commit. Returns |nil,
@@ -425,23 +524,15 @@ func GetCommittedValue(ctx context.Context, vr types.ValueReader, cv types.Value
 	return v, err
 }

-func parentsToQueue(ctx context.Context, refs types.RefSlice, q *RefByHeightHeap, vr types.ValueReader) error {
+func parentsToQueue(ctx context.Context, commits []*Commit, q *CommitByHeightHeap, vr types.ValueReader) error {
 	seen := make(map[hash.Hash]bool)
-	for _, r := range refs {
-		if _, ok := seen[r.TargetHash()]; ok {
+	for _, c := range commits {
+		if _, ok := seen[c.Addr()]; ok {
 			continue
 		}
-		seen[r.TargetHash()] = true
+		seen[c.Addr()] = true

-		v, err := r.TargetValue(ctx, vr)
-		if err != nil {
-			return err
-		}
-		if v == nil {
-			return fmt.Errorf("target not found: %v", r.TargetHash())
-		}
-
-		parents, err := GetCommitParents(ctx, vr, v)
+		parents, err := GetCommitParents(ctx, vr, c.NomsValue())
 		if err != nil {
 			return err
 		}
@@ -453,22 +544,22 @@ func parentsToQueue(ctx context.Context, refs types.RefSlice, q *RefByHeightHeap
 	return nil
 }

-func findCommonRef(a, b types.RefSlice) (types.Ref, bool) {
-	toRefSet := func(s types.RefSlice) map[hash.Hash]types.Ref {
-		out := map[hash.Hash]types.Ref{}
+func findCommonCommit(a, b []*Commit) (*Commit, bool) {
+	toAddrMap := func(s []*Commit) map[hash.Hash]*Commit {
+		out := map[hash.Hash]*Commit{}
 		for _, r := range s {
-			out[r.TargetHash()] = r
+			out[r.Addr()] = r
 		}
 		return out
 	}

-	aSet, bSet := toRefSet(a), toRefSet(b)
-	for s, r := range aSet {
-		if _, present := bSet[s]; present {
-			return r, true
+	as, bs := toAddrMap(a), toAddrMap(b)
+	for s, c := range as {
+		if _, present := bs[s]; present {
+			return c, true
 		}
 	}
-	return types.Ref{}, false
+	return nil, false
 }

 func makeCommitStructType(metaType, parentsType, parentsListType, parentsClosureType, valueType *types.Type, includeParentsClosure bool) (*types.Type, error) {
@@ -676,43 +767,46 @@ func IsCommit(v types.Value) (bool, error) {
 	}
 }

-type RefByHeightHeap []types.Ref
+type CommitByHeightHeap []*Commit

-func (r RefByHeightHeap) Less(i, j int) bool {
-	return types.HeightOrder(r[i], r[j])
+func (r CommitByHeightHeap) Less(i, j int) bool {
+	if r[i].Height() == r[j].Height() {
+		return r[i].Addr().Less(r[j].Addr())
+	}
+	return r[i].Height() > r[j].Height()
 }

-func (r RefByHeightHeap) Swap(i, j int) {
+func (r CommitByHeightHeap) Swap(i, j int) {
 	r[i], r[j] = r[j], r[i]
 }

-func (r RefByHeightHeap) Len() int {
+func (r CommitByHeightHeap) Len() int {
 	return len(r)
 }

-func (r *RefByHeightHeap) Push(x interface{}) {
-	*r = append(*r, x.(types.Ref))
+func (r *CommitByHeightHeap) Push(x interface{}) {
+	*r = append(*r, x.(*Commit))
 }

-func (r *RefByHeightHeap) Pop() interface{} {
+func (r *CommitByHeightHeap) Pop() interface{} {
 	old := *r
 	ret := old[len(old)-1]
 	*r = old[:len(old)-1]
 	return ret
 }

-func (r RefByHeightHeap) Empty() bool {
+func (r CommitByHeightHeap) Empty() bool {
 	return len(r) == 0
 }

-func (r RefByHeightHeap) MaxHeight() uint64 {
+func (r CommitByHeightHeap) MaxHeight() uint64 {
 	return r[0].Height()
 }

-func (r *RefByHeightHeap) PopRefsOfHeight(h uint64) types.RefSlice {
-	var ret types.RefSlice
+func (r *CommitByHeightHeap) PopCommitsOfHeight(h uint64) []*Commit {
+	var ret []*Commit
 	for !r.Empty() && r.MaxHeight() == h {
-		ret = append(ret, heap.Pop(r).(types.Ref))
+		ret = append(ret, heap.Pop(r).(*Commit))
 	}
 	return ret
 }
--- a/go/store/datas/commit_test.go
+++ b/go/store/datas/commit_test.go
@@ -289,17 +289,25 @@ 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 hash.Hash, ok bool, err error) {
-	closure, err := NewSetRefClosure(ctx, vr1, c1)
+func commonAncWithSetClosure(ctx context.Context, c1r, c2r types.Ref, vr1, vr2 types.ValueReader) (a hash.Hash, ok bool, err error) {
+	c1, err := commitPtrFromRef(ctx, vr1, c1r)
 	if err != nil {
 		return hash.Hash{}, false, err
 	}
-	return FindClosureCommonAncestor(ctx, closure, c2, vr2)
+	closure, err := NewSetCommitClosure(ctx, vr1, c1)
+	if err != nil {
+		return hash.Hash{}, false, err
+	}
+	return FindClosureCommonAncestor(ctx, closure, c2r, vr2)
 }

-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)
+func commonAncWithLazyClosure(ctx context.Context, c1r, c2r types.Ref, vr1, vr2 types.ValueReader) (a hash.Hash, ok bool, err error) {
+	c1, err := commitPtrFromRef(ctx, vr1, c1r)
+	if err != nil {
+		return hash.Hash{}, false, err
+	}
+	closure := NewLazyCommitClosure(c1, vr1)
+	return FindClosureCommonAncestor(ctx, closure, c2r, vr2)
 }

 // Assert that c is the common ancestor of a and b, using multiple common ancestor methods.
--- a/go/store/datas/ref_closure.go
+++ b/go/store/datas/ref_closure.go
@@ -21,43 +21,43 @@ import (
 	"github.com/dolthub/dolt/go/store/types"
 )

-// RefClosure is a transitive closure of types.Ref.
-type RefClosure interface {
-	// Contains returns true if |ref| is contained in the closure.
-	Contains(ctx context.Context, ref types.Ref) (bool, error)
+// CommitClosure is a transitive closure of commit parents.
+type CommitClosure interface {
+	// Contains returns true if |commit| is contained in the closure.
+	Contains(ctx context.Context, commit *Commit) (bool, error)
 }

-// NewSetRefClosure computes the entire transitive closure of |ref|.
-func NewSetRefClosure(ctx context.Context, vr types.ValueReader, ref types.Ref) (RefClosure, error) {
-	s, err := transitiveClosure(ctx, vr, ref)
+// NewSetCommitClosure computes the entire transitive closure of |commit|.
+func NewSetCommitClosure(ctx context.Context, vr types.ValueReader, commit *Commit) (CommitClosure, error) {
+	s, err := transitiveClosure(ctx, vr, commit)
 	if err != nil {
-		return setRefClosure{}, err
+		return setCommitClosure{}, err
 	}

-	return setRefClosure{HashSet: s}, nil
+	return setCommitClosure{HashSet: s}, nil
 }

-type setRefClosure struct {
+type setCommitClosure struct {
 	hash.HashSet
 }

-var _ RefClosure = setRefClosure{}
+var _ CommitClosure = setCommitClosure{}

-// Contains returns true if |ref| is contained in the closure.
-func (s setRefClosure) Contains(ctx context.Context, ref types.Ref) (ok bool, err error) {
-	ok = s.HashSet.Has(ref.TargetHash())
+// Contains returns true if |commit| is contained in the closure.
+func (s setCommitClosure) Contains(ctx context.Context, commit *Commit) (ok bool, err error) {
+	ok = s.HashSet.Has(commit.Addr())
 	return
 }

-func transitiveClosure(ctx context.Context, vr types.ValueReader, ref types.Ref) (s hash.HashSet, err error) {
-	h := &RefByHeightHeap{ref}
+func transitiveClosure(ctx context.Context, vr types.ValueReader, commit *Commit) (s hash.HashSet, err error) {
+	h := &CommitByHeightHeap{commit}
 	s = hash.NewHashSet()

-	var curr types.RefSlice
+	var curr []*Commit
 	for !h.Empty() {
-		curr = h.PopRefsOfHeight(h.MaxHeight())
-		for _, r := range curr {
-			s.Insert(r.TargetHash())
+		curr = h.PopCommitsOfHeight(h.MaxHeight())
+		for _, c := range curr {
+			s.Insert(c.Addr())
 		}

 		err = parentsToQueue(ctx, curr, h, vr)
@@ -69,44 +69,42 @@ func transitiveClosure(ctx context.Context, vr types.ValueReader, ref types.Ref)
 	return s, nil
 }

-// NewLazyRefClosure makes a lazy RefClosure, which computes the
-// transitive closure of |ref| on demand to answer Contains() queries.
-func NewLazyRefClosure(ref types.Ref, vr types.ValueReader) RefClosure {
-	return lazyRefClosure{
-		seen: hash.NewHashSet(ref.TargetHash()),
-		heap: &RefByHeightHeap{ref},
+// NewLazyCommitClosure makes a lazy CommitClosure, which computes the
+// transitive closure of |commit| on demand to answer Contains() queries.
+func NewLazyCommitClosure(commit *Commit, vr types.ValueReader) CommitClosure {
+	return lazyCommitClosure{
+		seen: hash.NewHashSet(commit.Addr()),
+		heap: &CommitByHeightHeap{commit},
 		vr:   vr,
 	}
 }

-type lazyRefClosure struct {
+type lazyCommitClosure struct {
 	seen hash.HashSet
-	heap *RefByHeightHeap
+	heap *CommitByHeightHeap
 	vr   types.ValueReader
 }

-var _ RefClosure = lazyRefClosure{}
+var _ CommitClosure = lazyCommitClosure{}

-// Contains returns true if |ref| is contained in the closure.
-func (l lazyRefClosure) Contains(ctx context.Context, ref types.Ref) (ok bool, err error) {
-	err = l.traverseBelowDepth(ctx, ref.Height())
+// Contains returns true if |commit| is contained in the closure.
+func (l lazyCommitClosure) Contains(ctx context.Context, commit *Commit) (ok bool, err error) {
+	err = l.traverseBelowDepth(ctx, commit.Height())
 	if err != nil {
 		return false, err
 	}
-	return l.seen.Has(ref.TargetHash()), nil
+	return l.seen.Has(commit.Addr()), nil
 }

 // traverseBelowDepth traverses through all of the refs of height |depth| or higher,
 // adding them to the set |l.seen|.
-func (l lazyRefClosure) traverseBelowDepth(ctx context.Context, depth uint64) (err error) {
-	var curr types.RefSlice
+func (l lazyCommitClosure) traverseBelowDepth(ctx context.Context, depth uint64) (err error) {
+	var curr []*Commit
 	for !l.heap.Empty() && depth <= l.heap.MaxHeight() {
-
-		curr = l.heap.PopRefsOfHeight(l.heap.MaxHeight())
+		curr = l.heap.PopCommitsOfHeight(l.heap.MaxHeight())
 		for _, r := range curr {
-			l.seen.Insert(r.TargetHash())
+			l.seen.Insert(r.Addr())
 		}
-
 		err = parentsToQueue(ctx, curr, l.heap, l.vr)
 		if err != nil {
 			return err