More testing, less bugs and TODOs

go/cmd/dolt/commands/engine/sql_print.go

@@ -20,7 +20,6 @@ import (
 	"io"
 	"time"
 
-	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/dolthub/go-mysql-server/sql/types"
 
@@ -158,7 +157,7 @@ func writeResultSet(ctx *sql.Context, rowIter sql.RowIter, wr table.SqlRowWriter
 	i := 0
 	for {
 		r, err := rowIter.Next(ctx)
-		if err == io.EOF || err == doltdb.ErrGhostCommitEncountered {
+		if err == io.EOF {
 			break
 		} else if err != nil {
 			return 0, err

go/libraries/doltcore/cherry_pick/cherry_pick.go

@@ -209,7 +209,7 @@ func cherryPick(ctx *sql.Context, dSess *dsess.DoltSession, roots doltdb.Roots,
 		return nil, "", err
 	}
 	cherryCommit, ok := optCmt.ToCommit()
-	if err != nil {
+	if !ok {
 		return nil, "", doltdb.ErrGhostCommitEncountered
 	}
 

go/libraries/doltcore/dbfactory/file.go

@@ -169,7 +169,7 @@ func (fact FileFactory) CreateDB(ctx context.Context, nbf *types.NomsBinFormat,
 	}
 
 	// NM4 - error handling. Currently, no-op (nil) if there is no hardcoded file.
-	lostGen := nbs.BuildGhostBlockStore(path)
+	lostGen := nbs.NewGhostBlockStore(path)
 
 	st := nbs.NewGenerationalCS(oldGenSt, newGenSt, lostGen)
 	// metrics?

go/libraries/doltcore/doltdb/commit.go

@@ -70,7 +70,9 @@ var _ Rootish = &Commit{}
 
 // NewCommit generates a new Commit object that wraps a supplied datas.Commit.
 func NewCommit(ctx context.Context, vrw types.ValueReadWriter, ns tree.NodeStore, commit *datas.Commit) (*Commit, error) {
-	commit.IsGhost()
+	if commit.IsGhost() {
+		return nil, ErrGhostCommitRuntimeFailure
+	}
 
 	parents, err := datas.GetCommitParents(ctx, vrw, commit.NomsValue())
 	if err != nil {
@@ -280,7 +282,7 @@ func (c *Commit) GetAncestor(ctx context.Context, as *AncestorSpec) (*OptionalCo
 		var ok bool
 		hardInst, ok = optInst.ToCommit()
 		if !ok {
-			return nil, ErrGhostCommitEncountered
+			break
 		}
 	}
 

go/libraries/doltcore/doltdb/doltdb.go

@@ -1935,6 +1935,9 @@ func (ddb *DoltDB) GetStashRootAndHeadCommitAtIdx(ctx context.Context, idx int)
 	return getStashAtIdx(ctx, ds, ddb.vrw, ddb.NodeStore(), idx)
 }
 
+// PersistGhostCommits persists the set of ghost commits to the database. This is how the application layer passes
+// information about ghost commits to the storage layer. This can be called multiple times over the course of performing
+// a shallow clone, but should not be called after the clone is complete.
 func (ddb *DoltDB) PersistGhostCommits(ctx context.Context, ghostCommits hash.HashSet) error {
-	return ddb.db.Database.WriteDemGhosts(ctx, ghostCommits)
+	return ddb.db.Database.PersistGhostCommitIDs(ctx, ghostCommits)
 }

go/libraries/doltcore/env/actions/remotes.go

@@ -600,7 +600,8 @@ func fetchRefSpecsWithDepth(
 		}
 		commit, ok := optCmt.ToCommit()
 		if !ok {
-			return doltdb.ErrGhostCommitEncountered // NM4 - This definitely needs its own error message. TEST THIS PATH.
+			// Source DB should have everything. If we can't read a commit, something is wrong.
+			return doltdb.ErrGhostCommitRuntimeFailure
 		}
 
 		remoteTrackRef := newHead.Ref
@@ -650,9 +651,8 @@ func fetchRefSpecsWithDepth(
 }
 
 func buildInitialSkipList(ctx context.Context, srcDB *doltdb.DoltDB, toFetch []hash.Hash) (hash.HashSet, error) {
-	// if toFetch is more than one, panic. NM4 - test this path. shouldn't panic, obviously.
 	if len(toFetch) > 1 {
-		panic("multi head shallow pulls not implemented")
+		return hash.HashSet{}, fmt.Errorf("runtime error: multiple refspect not supported in shallow clone")
 	}
 
 	cs, err := doltdb.NewCommitSpec(toFetch[0].String())

go/libraries/doltcore/merge/action.go

@@ -106,7 +106,8 @@ func NewMergeSpec(
 	}
 	headCM, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, doltdb.ErrGhostCommitEncountered // NM4 - need better message. TEST THIS PATH.
+		// HEAD should always resolve to a commit, so this should never happen.
+		return nil, doltdb.ErrGhostCommitRuntimeFailure
 	}
 
 	mergeCS, err := doltdb.NewCommitSpec(commitSpecStr)
@@ -120,7 +121,7 @@ func NewMergeSpec(
 	}
 	mergeCM, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, doltdb.ErrGhostCommitEncountered // NM4 - need better message. TEST THIS PATH
+		return nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	headH, err := headCM.HashOf()

go/libraries/doltcore/merge/merge.go

@@ -54,7 +54,8 @@ func MergeCommits(ctx *sql.Context, commit, mergeCommit *doltdb.Commit, opts edi
 	}
 	ancCommit, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, doltdb.ErrGhostCommitEncountered // NM4 - not sure if we can get to this point. TEST THIS PATH.
+		// Ancestor commit should have been resolved before getting this far.
+		return nil, doltdb.ErrGhostCommitRuntimeFailure
 	}
 
 	ourRoot, err := commit.GetRootValue(ctx)

go/libraries/doltcore/merge/merge_base.go

@@ -29,7 +29,7 @@ func MergeBase(ctx context.Context, left, right *doltdb.Commit) (base hash.Hash,
 	}
 	ancestor, ok := optCmt.ToCommit()
 	if !ok {
-		return base, doltdb.ErrGhostCommitEncountered // NM4 - TEST.  I think getCommitAncestor is going to be an awk one.
+		return base, doltdb.ErrGhostCommitEncountered
 	}
 
 	return ancestor.HashOf()

go/libraries/doltcore/merge/revert.go

@@ -66,7 +66,7 @@ func Revert(ctx *sql.Context, ddb *doltdb.DoltDB, root *doltdb.RootValue, commit
 		}
 		parentCM, ok := optCmt.ToCommit()
 		if !ok {
-			return nil, "", doltdb.ErrGhostCommitEncountered // NM4 - TEST THIS PATH.
+			return nil, "", doltdb.ErrGhostCommitEncountered
 		}
 
 		theirRoot, err := parentCM.GetRootValue(ctx)

go/libraries/doltcore/rebase/filter_branch.go

@@ -190,7 +190,7 @@ func rebaseRecursive(ctx context.Context, ddb *doltdb.DoltDB, replay ReplayCommi
 	for _, optParent := range allOptParents {
 		parent, ok := optParent.ToCommit()
 		if !ok {
-			return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST, surely needs a better error.
+			return nil, doltdb.ErrGhostCommitEncountered
 		}
 		allParents = append(allParents, parent)
 	}

go/libraries/doltcore/remotestorage/chunk_store.go

@@ -122,11 +122,6 @@ type DoltChunkStore struct {
 	wsValidate  bool
 }
 
-func (dcs *DoltChunkStore) GhostTheseRefsBrah(ctx context.Context, refs hash.HashSet) error {
-	//TODO implement me
-	panic("NM4")
-}
-
 func NewDoltChunkStoreFromPath(ctx context.Context, nbf *types.NomsBinFormat, path, host string, wsval bool, csClient remotesapi.ChunkStoreServiceClient) (*DoltChunkStore, error) {
 	var repoId *remotesapi.RepoId
 
@@ -956,6 +951,10 @@ func (dcs *DoltChunkStore) StatsSummary() string {
 	return fmt.Sprintf("CacheHits: %v", dcs.Stats().(CacheStats).CacheHits())
 }
 
+func (dcs *DoltChunkStore) PersistGhostHashes(ctx context.Context, refs hash.HashSet) error {
+	panic("runtime error: PersistGhostHashes should never be called on a remote chunk store")
+}
+
 // Close tears down any resources in use by the implementation. After
 // Close(), the ChunkStore may not be used again. It is NOT SAFE to call
 // Close() concurrently with any other ChunkStore method; behavior is

go/libraries/doltcore/sqle/database.go

@@ -511,7 +511,7 @@ func resolveAsOfTime(ctx *sql.Context, ddb *doltdb.DoltDB, head ref.DoltRef, asO
 	}
 	cm, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, nil, doltdb.ErrGhostCommitEncountered // NM4 - NEED TEST.
+		return nil, nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	h, err := cm.HashOf()
@@ -533,7 +533,7 @@ func resolveAsOfTime(ctx *sql.Context, ddb *doltdb.DoltDB, head ref.DoltRef, asO
 		}
 		curr, ok := optCmt.ToCommit()
 		if !ok {
-			return nil, nil, doltdb.ErrGhostCommitEncountered // NM4 - NEED TEST.
+			return nil, nil, doltdb.ErrGhostCommitEncountered
 		}
 
 		meta, err := curr.GetCommitMeta(ctx)
@@ -586,7 +586,7 @@ func resolveAsOfCommitRef(ctx *sql.Context, db Database, head ref.DoltRef, commi
 	}
 	cm, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, nil, doltdb.ErrGhostCommitEncountered // NM4 - NEED TEST.
+		return nil, nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	root, err := cm.GetRootValue(ctx)

go/libraries/doltcore/sqle/database_provider.go

@@ -1087,7 +1087,7 @@ func resolveAncestorSpec(ctx *sql.Context, revSpec string, ddb *doltdb.DoltDB) (
 	ok := false
 	cm, ok = optCmt.ToCommit()
 	if !ok {
-		return "", doltdb.ErrGhostCommitEncountered // NM4 - NEED TEST.
+		return "", doltdb.ErrGhostCommitEncountered
 	}
 
 	hash, err := cm.HashOf()
@@ -1482,7 +1482,7 @@ func initialStateForCommit(ctx context.Context, srcDb ReadOnlyDatabase) (dsess.I
 	}
 	cm, ok := optCmt.ToCommit()
 	if !ok {
-		return dsess.InitialDbState{}, doltdb.ErrGhostCommitEncountered // NM4 - NEED TEST???
+		return dsess.InitialDbState{}, doltdb.ErrGhostCommitEncountered
 	}
 
 	init := dsess.InitialDbState{

go/libraries/doltcore/sqle/dfunctions/dolt_merge_base.go

@@ -105,7 +105,7 @@ func resolveRefSpecs(ctx *sql.Context, leftSpec, rightSpec string) (left, right
 	}
 	left, ok = optCmt.ToCommit()
 	if !ok {
-		return nil, nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST, surely needs a better error.
+		return nil, nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	optCmt, err = doltDB.Resolve(ctx, rcs, headRef)
@@ -114,7 +114,7 @@ func resolveRefSpecs(ctx *sql.Context, leftSpec, rightSpec string) (left, right
 	}
 	right, ok = optCmt.ToCommit()
 	if !ok {
-		return nil, nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST, surely needs a better error.
+		return nil, nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	return

go/libraries/doltcore/sqle/dfunctions/hashof.go

@@ -91,7 +91,7 @@ func (t *HashOf) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 				}
 				cm, ok = optCmt.ToCommit()
 				if !ok {
-					return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST this.
+					return nil, doltdb.ErrGhostCommitEncountered
 				}
 			} else {
 				return nil, err

go/libraries/doltcore/sqle/dolt_diff_table_function.go

@@ -337,7 +337,7 @@ func resolveCommit(ctx *sql.Context, ddb *doltdb.DoltDB, headRef ref.DoltRef, cS
 	}
 	cm, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, doltdb.ErrGhostCommitEncountered // NM4 - NEED TEST.
+		return nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	return cm, nil

go/libraries/doltcore/sqle/dolt_log_table_function.go

@@ -443,7 +443,7 @@ func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter
 		}
 		commit, ok = optCmt.ToCommit()
 		if err != nil {
-			return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+			return nil, doltdb.ErrGhostCommitEncountered
 		}
 
 		commits = append(commits, commit)
@@ -462,7 +462,7 @@ func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter
 		}
 		notCommit, ok := optCmt.ToCommit()
 		if !ok {
-			return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+			return nil, doltdb.ErrGhostCommitEncountered
 		}
 
 		notCommits = append(notCommits, notCommit)
@@ -486,7 +486,7 @@ func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter
 		}
 		mergeCommit, ok := optCmt.ToCommit()
 		if !ok {
-			return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+			return nil, doltdb.ErrGhostCommitEncountered
 		}
 
 		notCommits = append(notCommits, mergeCommit)
@@ -678,7 +678,7 @@ func (itr *logTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) {
 		ok := false
 		commit, ok = optCmt.ToCommit()
 		if !ok {
-			return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+			return nil, doltdb.ErrGhostCommitEncountered
 		}
 
 		if itr.tableNames != nil {
@@ -693,7 +693,7 @@ func (itr *logTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) {
 			}
 			parent0Cm, ok := optCmt.ToCommit()
 			if !ok {
-				return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+				return nil, doltdb.ErrGhostCommitEncountered
 			}
 
 			var parent1Cm *doltdb.Commit
@@ -704,7 +704,7 @@ func (itr *logTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) {
 				}
 				parent1Cm, ok = optCmt.ToCommit()
 				if !ok {
-					return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+					return nil, doltdb.ErrGhostCommitEncountered
 				}
 			}
 

go/libraries/doltcore/sqle/dprocedures/dolt_count_commits.go

@@ -82,7 +82,7 @@ func countCommits(ctx *sql.Context, args ...string) (ahead uint64, behind uint64
 	}
 	fromCommit, ok := optCmt.ToCommit()
 	if !ok {
-		return 0, 0, doltdb.ErrGhostCommitEncountered // NM4 - Have no idea what this is
+		return 0, 0, doltdb.ErrGhostCommitEncountered
 	}
 
 	fromHash, err := fromCommit.HashOf()
@@ -100,7 +100,7 @@ func countCommits(ctx *sql.Context, args ...string) (ahead uint64, behind uint64
 	}
 	toCommit, ok := optCmt.ToCommit()
 	if !ok {
-		return 0, 0, doltdb.ErrGhostCommitEncountered // NM4 - Have no idea what this is
+		return 0, 0, doltdb.ErrGhostCommitEncountered
 	}
 
 	toHash, err := toCommit.HashOf()
@@ -114,7 +114,7 @@ func countCommits(ctx *sql.Context, args ...string) (ahead uint64, behind uint64
 	}
 	ancestor, ok := optCmt.ToCommit()
 	if !ok {
-		return 0, 0, doltdb.ErrGhostCommitEncountered // NM4 - Have no idea what this is
+		return 0, 0, doltdb.ErrGhostCommitEncountered
 	}
 
 	ancestorHash, err := ancestor.HashOf()

go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go

@@ -199,7 +199,7 @@ func startRebase(ctx *sql.Context, upstreamPoint string) error {
 	}
 	upstreamCommit, ok := optCmt.ToCommit()
 	if !ok {
-		return doltdb.ErrGhostCommitEncountered // NM4 - prob user input. TEST. message.
+		return doltdb.ErrGhostCommitEncountered
 	}
 
 	// rebaseWorkingBranch is the name of the temporary branch used when performing a rebase. In Git, a rebase
@@ -565,7 +565,7 @@ func squashCommitMessage(ctx *sql.Context, nextCommitHash string) (string, error
 	}
 	nextCommit, ok := optCmt.ToCommit()
 	if !ok {
-		return "", doltdb.ErrGhostCommitEncountered // NM4 - TEST.
+		return "", doltdb.ErrGhostCommitEncountered
 	}
 
 	nextCommitMeta, err := nextCommit.GetCommitMeta(ctx)

go/libraries/doltcore/sqle/dprocedures/dolt_revert.go

@@ -104,7 +104,7 @@ func doDoltRevert(ctx *sql.Context, args []string) (int, error) {
 		}
 		commit, ok := optCmt.ToCommit()
 		if !ok {
-			return 1, doltdb.ErrGhostCommitEncountered // NM4 TEST - I think should never happen.
+			return 1, doltdb.ErrGhostCommitEncountered
 		}
 
 		commits[i] = commit

go/libraries/doltcore/sqle/dsess/session.go

@@ -697,7 +697,7 @@ func (d *DoltSession) newPendingCommit(ctx *sql.Context, branchState *branchStat
 			}
 			parentCommit, ok := optCmt.ToCommit()
 			if !ok {
-				return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST THIS PATH.
+				return nil, doltdb.ErrGhostCommitEncountered
 			}
 
 			mergeParentCommits = append(mergeParentCommits, parentCommit)
@@ -897,7 +897,7 @@ func (d *DoltSession) ResolveRootForRef(ctx *sql.Context, dbName, refStr string)
 	}
 	cm, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, nil, "", doltdb.ErrGhostCommitEncountered
+		return nil, nil, "", doltdb.ErrGhostCommitRuntimeFailure
 	}
 
 	root, err = cm.GetRootValue(ctx)

go/libraries/doltcore/sqle/dsess/transactions.go

@@ -208,7 +208,7 @@ func doltCommit(ctx *sql.Context,
 	}
 	curHead, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, nil, doltdb.ErrGhostCommitEncountered // NM4 - def runtime error.
+		return nil, nil, doltdb.ErrGhostCommitRuntimeFailure
 	}
 
 	// We already got a new staged root via merge or ff via the doCommit method, so now apply it to the STAGED value

go/libraries/doltcore/sqle/dtables/column_diff_table.go

@@ -342,7 +342,7 @@ func (itr *doltColDiffCommitHistoryRowItr) Next(ctx *sql.Context) (sql.Row, erro
 			}
 			commit, ok := optCmt.ToCommit()
 			if !ok {
-				return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST, message.
+				return nil, doltdb.ErrGhostCommitEncountered
 			}
 
 			err = itr.loadTableChanges(ctx, commit)
@@ -420,7 +420,7 @@ func (itr *doltColDiffCommitHistoryRowItr) calculateTableChanges(ctx context.Con
 	}
 	parent, ok := optCmt.ToCommit()
 	if !ok {
-		return nil, doltdb.ErrGhostCommitEncountered // NM4 - TEST, message.
+		return nil, doltdb.ErrGhostCommitEncountered
 	}
 
 	fromRootValue, err := parent.GetRootValue(ctx)