ronantakizawa/codereview-bench huggingface dataset:
Repository: dolthub/dolt
File: go/store/nbs/file_table_reader.go (Go)

Code before:
type fileReaderAt struct {
	f    *os.File
	path string
	sz   int64
	// refcnt, clone() increments and Close() decrements. The *os.File is closed when it reaches 0.
	cnt *int32
}

func (fra *fileReaderAt) clone() (tableReaderAt, error) {
	if atomic.AddInt32(fra.cnt, 1) == 1 {
		panic("attempt to clone a closed fileReaderAt")
	}
	return &fileReaderAt{
		fra.f,
		fra.path,
		fra.sz,
		fra.cnt,
	}, nil
}

func (fra *fileReaderAt) Close() error {
	cnt := atomic.AddInt32(fra.cnt, -1)
	if cnt == 0 {
		return fra.f.Close()
	} else if cnt < 0 {
		panic("invalid cnt on fileReaderAt")
	} else {
		return nil
	}
}

func (fra *fileReaderAt) Reader(ctx context.Context) (io.ReadCloser, error) {
	return os.Open(fra.path)
}

func (fra *fileReaderAt) ReadAtWithStats(ctx context.Context, p []byte, off int64, stats *Stats) (n int, err error) {
	t1 := time.Now()
	defer func() {
		stats.FileBytesPerRead.Sample(uint64(len(p)))
		stats.FileReadLatency.SampleTimeSince(t1)
	}()
	return fra.f.ReadAt(p, off)
}

func newTableFileMetadata(path string, chunkCount uint32) (*TableFileMetadata, error) {
	fra, err := newFileReaderAt(path)
	if err != nil {
		return nil, err
	}

	idxSz := int64(indexSize(chunkCount) + footerSize)

Review comment (bug):
Also, why panic? Yeah, it's a bug, but is it worth bringing down the server? Could we just put an error in the logs and move on?

Code after:

type fileReaderAt struct {
	f    *os.File
	path string
	sz   int64
	// refcnt, clone() increments and Close() decrements. The *os.File is closed when it reaches 0.
	cnt *int32
}

func (fra *fileReaderAt) clone() (tableReaderAt, error) {
	if dynassert.Assert(atomic.AddInt32(fra.cnt, 1) > 1, "attempt to clone a closed fileReaderAt") {
		// Restore previous refcnt, even know we're in a weird state...
		atomic.AddInt32(fra.cnt, -1)
		return newFileReaderAt(fra.path)
	}
	return &fileReaderAt{
		fra.f,
		fra.path,
		fra.sz,
		fra.cnt,
	}, nil
}

func (fra *fileReaderAt) Close() error {
	cnt := atomic.AddInt32(fra.cnt, -1)
	dynassert.Assert(cnt >= 0, "invalid cnt on fileReaderAt")
	if cnt == 0 {
		return fra.f.Close()
	}
	return nil
}

func (fra *fileReaderAt) Reader(ctx context.Context) (io.ReadCloser, error) {
	return os.Open(fra.path)
}

func (fra *fileReaderAt) ReadAtWithStats(ctx context.Context, p []byte, off int64, stats *Stats) (n int, err error) {
	t1 := time.Now()
	defer func() {
		stats.FileBytesPerRead.Sample(uint64(len(p)))
		stats.FileReadLatency.SampleTimeSince(t1)
	}()
	return fra.f.ReadAt(p, off)
}

func newTableFileMetadata(path string, chunkCount uint32) (*TableFileMetadata, error) {
	fra, err := newFileReaderAt(path)
	if err != nil {
		return nil, err
	}
Repository: dolthub/dolt
File: go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go (Go)

Code before:

	enginetest.TestPersist(t, harness, newPersistableSession)
}

func TestTypesOverWire(t *testing.T) {
	harness := newDoltHarness(t)
	defer harness.Close()
	enginetest.TestTypesOverWire(t, harness, newSessionBuilder(harness))
}

func TestDoltCherryPick(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCherryPickTests(t, harness)
}

func TestDoltCherryPickPrepared(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCherryPickTestsPrepared(t, harness)
}

func TestDoltCommit(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCommitTests(t, harness)
}

func TestAmendInitialDoltCommit(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunAmendInitialDoltCommitTests(t, harness)
}

func TestDoltCommitPrepared(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCommitTestsPrepared(t, harness)
}

func TestQueriesPrepared(t *testing.T) {
	h := newDoltHarness(t)
	defer h.Close()
	enginetest.TestQueriesPrepared(t, h)
}

func TestStatsHistograms(t *testing.T) {
	h := newDoltEnginetestHarness(t)
	RunStatsHistogramTests(t, h)
}

// TestStatsIO force a provider reload in-between setup and assertions that
// forces a round trip of the statistics table before inspecting values.
func TestStatsStorage(t *testing.T) {
	h := newDoltEnginetestHarness(t)
	RunStatsStorageTests(t, h)

Review comment (suggestion):
Drop this. Use the existing `DoltCommitTests` array. And while you are at it, move all of those tests into a new file called dolt_queries_commit.go

Code after:

	enginetest.TestPersist(t, harness, newPersistableSession)
}

func TestTypesOverWire(t *testing.T) {
	harness := newDoltHarness(t)
	defer harness.Close()
	enginetest.TestTypesOverWire(t, harness, newSessionBuilder(harness))
}

func TestDoltCherryPick(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCherryPickTests(t, harness)
}

func TestDoltCherryPickPrepared(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCherryPickTestsPrepared(t, harness)
}

func TestDoltCommit(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCommitTests(t, harness)
}

func TestDoltCommitPrepared(t *testing.T) {
	harness := newDoltEnginetestHarness(t)
	RunDoltCommitTestsPrepared(t, harness)
}

func TestQueriesPrepared(t *testing.T) {
	h := newDoltHarness(t)
	defer h.Close()
	enginetest.TestQueriesPrepared(t, h)
}

func TestStatsHistograms(t *testing.T) {
	h := newDoltEnginetestHarness(t)
	RunStatsHistogramTests(t, h)
}

// TestStatsIO force a provider reload in-between setup and assertions that
// forces a round trip of the statistics table before inspecting values.
func TestStatsStorage(t *testing.T) {
	h := newDoltEnginetestHarness(t)
	RunStatsStorageTests(t, h)
}

func TestJoinStats(t *testing.T) {
	h := newDoltEnginetestHarness(t)
	RunJoinStatsTests(t, h)
Repository: dolthub/dolt
File: go/store/datas/database_common.go (Go)

Code before:
			// calculated for the newWSHash.
			ae := am.Editor()
			err = ae.Update(ctx, ds.ID(), h)
			if err != nil {
				return prolly.AddressMap{}, err
			}

			if workingSetPath != "" && newWSHash != (hash.Hash{}) {
				err = ae.Update(ctx, workingSetPath, newWSHash)
				if err != nil {
					return prolly.AddressMap{}, err
				}
			}

			return ae.Flush(ctx)
		})

	if err == ErrAlreadyCommitted {
		return nil
	}

	return err
}

func (db *database) BuildNewCommit(ctx context.Context, ds Dataset, v types.Value, opts CommitOptions) (*Commit, error) {
	if len(opts.Parents) == 0 && !opts.Amend {
		headAddr, ok := ds.MaybeHeadAddr()
		if ok {
			opts.Parents = []hash.Hash{headAddr}
		}
	} else if !opts.Amend {
		curr, ok := ds.MaybeHeadAddr()
		if ok {
			if !hasParentHash(opts, curr) {
				return nil, ErrMergeNeeded
			}
		}
	}

	return newCommitForValue(ctx, ds.db.chunkStore(), ds.db, ds.db.nodeStore(), v, opts)
}

func (db *database) Commit(ctx context.Context, ds Dataset, v types.Value, opts CommitOptions) (Dataset, error) {
	commit, err := db.BuildNewCommit(ctx, ds, v, opts)
	if err != nil {
		return Dataset{}, err
	}
	return db.WriteCommit(ctx, ds, commit)
}

func (db *database) WriteCommit(ctx context.Context, ds Dataset, commit *Commit) (Dataset, error) {

Review comment (suggestion):
This is logically equivalent, and makes it a little more clear what is happening:
```go
	if !opts.Amend {
		headAddr, ok := ds.MaybeHeadAddr()
		if ok {
			if len(opts.Parents) == 0 {
				opts.Parents = []hash.Hash{headAddr}
			} else {
				if !hasParentHash(opts, headAddr) {
					return nil, ErrMergeNeeded
				}
			}
		}
	}
```

Code after:
			// calculated for the newWSHash.
			ae := am.Editor()
			err = ae.Update(ctx, ds.ID(), h)
			if err != nil {
				return prolly.AddressMap{}, err
			}

			if workingSetPath != "" && newWSHash != (hash.Hash{}) {
				err = ae.Update(ctx, workingSetPath, newWSHash)
				if err != nil {
					return prolly.AddressMap{}, err
				}
			}

			return ae.Flush(ctx)
		})

	if err == ErrAlreadyCommitted {
		return nil
	}

	return err
}

func (db *database) BuildNewCommit(ctx context.Context, ds Dataset, v types.Value, opts CommitOptions) (*Commit, error) {
	if !opts.Amend {
		headAddr, ok := ds.MaybeHeadAddr()
		if ok {
			if len(opts.Parents) == 0 {
				opts.Parents = []hash.Hash{headAddr}
			} else {
				if !hasParentHash(opts, headAddr) {
					return nil, ErrMergeNeeded
				}
			}
		}
	}

	return newCommitForValue(ctx, ds.db.chunkStore(), ds.db, ds.db.nodeStore(), v, opts)
}

func (db *database) Commit(ctx context.Context, ds Dataset, v types.Value, opts CommitOptions) (Dataset, error) {
	commit, err := db.BuildNewCommit(ctx, ds, v, opts)
	if err != nil {
		return Dataset{}, err
	}
	return db.WriteCommit(ctx, ds, commit)
}

func (db *database) WriteCommit(ctx context.Context, ds Dataset, commit *Commit) (Dataset, error) {
	currentAddr, _ := ds.MaybeHeadAddr()
Repository: dolthub/dolt
File: go/cmd/dolt/commands/sqlserver/server.go (Go)

Code before:
				if conflict != nil && !conflict.IsEphemeral {
					warn.WriteString(fmt.Sprintf("Dropped persistent '%s@%s' as it conflicts with dolt reserved '%s@%s'", conflict.User, conflict.Host, user, host))
					conflicts = append(conflicts, mysql_db.UserPrimaryKey{Host: conflict.Host, User: conflict.User})
				}
			}
			rd.Close()

			ed := mysqlDb.Editor()
			defer ed.Close()

			for _, conflict := range conflicts {
				ed.RemoveUser(conflict)

				ed.RemoveRoleEdgesFromKey(mysql_db.RoleEdgesFromKey{
					FromHost: conflict.Host,
					FromUser: conflict.User,
				})

				ed.RemoveRoleEdgesToKey(mysql_db.RoleEdgesToKey{
					ToHost: conflict.Host,
					ToUser: conflict.User,
				})
			}

			if len(conflicts) > 0 {
				log.Warning(warn.String())
				sqlCtx, err := sqlEngine.NewDefaultContext(ctx)
				if err != nil {
					return fmt.Errorf("failed to create SQL context: %v", err)
				}

				if err := mysqlDb.Persist(sqlCtx, ed); err != nil {
					return fmt.Errorf("failed to persist changes to privileges database: %v", err)
				}
			}

			mysqlDb.AddEphemeralSuperUser(ed, LocalConnectionUser, "localhost", localCreds.Secret)

			return nil
		},
	}
	controller.Register(InitLockSuperUser)

	DisableMySQLDbIfRequired := &svcs.AnonService{
		InitF: func(context.Context) error {
			if ExternalDisableUsers {
				mysqlDb := sqlEngine.GetUnderlyingEngine().Analyzer.Catalog.MySQLDb
				mysqlDb.SetEnabled(false)
			}
			return nil
		},

Review comment (suggestion):
The `log` package from Vitess probably happens to work, but it's a bit indirect. There is already code in this file that uses `logrus.Warn()` to log warnings, so just stick with that to be consistent with the existing logging code. 

Code after:
				conflicts = append(conflicts, mysql_db.UserPrimaryKey{Host: conflict.Host, User: conflict.User})
			}

			ed := mysqlDb.Editor()
			defer ed.Close()

			if len(conflicts) > 0 {
				c := conflicts[0]
				ed.RemoveUser(c)
				ed.RemoveRoleEdgesFromKey(mysql_db.RoleEdgesFromKey{
					FromHost: c.Host,
					FromUser: c.User,
				})
				ed.RemoveRoleEdgesToKey(mysql_db.RoleEdgesToKey{
					ToHost: c.Host,
					ToUser: c.User,
				})

				sqlCtx, err := sqlEngine.NewDefaultContext(ctx)
				if err != nil {
					return fmt.Errorf("failed to create SQL context: %v", err)
				}
				if err := mysqlDb.Persist(sqlCtx, ed); err != nil {
					return fmt.Errorf("failed to persist changes to privileges database: %v", err)
				}
			}

			mysqlDb.AddEphemeralSuperUser(ed, LocalConnectionUser, "localhost", localCreds.Secret)

			return nil
		},
	}
	controller.Register(InitLockSuperUser)

	DisableMySQLDbIfRequired := &svcs.AnonService{
		InitF: func(context.Context) error {
			if ExternalDisableUsers {
				mysqlDb := sqlEngine.GetUnderlyingEngine().Analyzer.Catalog.MySQLDb
				mysqlDb.SetEnabled(false)
			}
			return nil
		},
	}
	controller.Register(DisableMySQLDbIfRequired)

	type SQLMetricsService struct {
		state svcs.ServiceState
		lis   net.Listener
		srv   *http.Server
	}
Repository: dolthub/dolt
File: go/libraries/doltcore/sqle/enginetest/dolt_queries_rm.go (Go)

Code before:
				ExpectedErrStr: "Nothing specified, nothing removed. Which tables should I remove?",
			},
		},
	},
	{
		Name: "simple dolt Rm",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_COMMIT('-A', '-m', 'created table')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:    "call dolt_rm('test');",
				Expected: []sql.Row{{0}},
			},
			{
				Query:          "select * from test;",
				ExpectedErrStr: "table not found: test",
			},
		},
	},
	{
		Name: "dolt Rm staged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_ADD('.')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test have changes saved in the index.",
			},
		},
	},
	{
		Name: "dolt Rm unstaged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test do not exist",
			},
		},
	},
	{
		Name: "dolt Rm with cached option",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"INSERT INTO test VALUES (1)",

Review comment (suggestion):
always be as specific as possible unless you are testing being unspecific.

So: `CALL DOLT_ADD('test')`

Code after:
				ExpectedErrStr: "unable to rm in read-only databases",
			},
		},
	},
}

var DoltRmTests = []queries.ScriptTest{
	{
		Name: "dolt Rm without tables",
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm();",
				ExpectedErrStr: "Nothing specified, nothing removed. Which tables should I remove?",
			},
		},
	},
	{
		Name: "simple dolt Rm",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_COMMIT('-A', '-m', 'created table')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:    "call dolt_rm('test');",
				Expected: []sql.Row{{0}},
			},
			{
				Query:          "select * from test;",
				ExpectedErrStr: "table not found: test",
			},
		},
	},
	{
		Name: "dolt Rm staged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_ADD('test')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test have changes saved in the index. Use --cached or commit.",
			},
		},
	},
	{
		Name: "dolt Rm unstaged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
		},
Repository: dolthub/dolt
File: go/libraries/doltcore/sqle/enginetest/dolt_queries_rm.go (Go)

Code before:
				ExpectedErrStr: "table not found: test",
			},
		},
	},
	{
		Name: "dolt Rm staged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_ADD('.')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test have changes saved in the index.",
			},
		},
	},
	{
		Name: "dolt Rm unstaged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test do not exist",
			},
		},
	},
	{
		Name: "dolt Rm with cached option",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"INSERT INTO test VALUES (1)",
			"CALL DOLT_COMMIT('-A', '-m', 'created table')",
			"call dolt_rm('test', '--cached');",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:    "select * from test;",
				Expected: []sql.Row{{1}},
			},
		},
	},
	{
		Name: "dolt Rm staged table with cached option",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_ADD('.')",
		},
		Assertions: []queries.ScriptTestAssertion{

Review comment (suggestion):
This error message isn't really correct. The table does exist, it just hasn't been staged or committed.

Code after:
		Name: "simple dolt Rm",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_COMMIT('-A', '-m', 'created table')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:    "call dolt_rm('test');",
				Expected: []sql.Row{{0}},
			},
			{
				Query:          "select * from test;",
				ExpectedErrStr: "table not found: test",
			},
		},
	},
	{
		Name: "dolt Rm staged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_ADD('test')",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test have changes saved in the index. Use --cached or commit.",
			},
		},
	},
	{
		Name: "dolt Rm unstaged table",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
				ExpectedErrStr: "error: the table(s) test have unstaged changes.",
			},
		},
	},
	{
		Name: "dolt Rm staged table with unstaged changes",
		SetUpScript: []string{
			"CREATE TABLE test (i int)",
			"CALL DOLT_ADD('test')",
			"INSERT INTO test VALUES (1)",
		},
		Assertions: []queries.ScriptTestAssertion{
			{
				Query:          "call dolt_rm('test');",
Repository: dolthub/dolt
File: go/cmd/dolt/commands/admin/newgen_to_oldgen.go (Go)

Code before:
// Copyright 2024 Dolthub, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package admin

import (
	"context"
	"os"
	"sync"
	"time"

	"github.com/dolthub/dolt/go/cmd/dolt/cli"
	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
	"github.com/dolthub/dolt/go/libraries/doltcore/env"
	"github.com/dolthub/dolt/go/libraries/utils/argparser"

Review comment (suggestion):
```suggestion
// Copyright 2025 Dolthub, Inc.
```

Code after:
// Copyright 2025 Dolthub, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package admin

import (
	"context"
	"os"
	"time"

	"golang.org/x/sync/errgroup"

	"github.com/dolthub/dolt/go/cmd/dolt/cli"
	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
	"github.com/dolthub/dolt/go/libraries/doltcore/env"
Repository: dolthub/dolt
File: go/libraries/doltcore/env/actions/remotes.go (Go)

Code before:
			wg, statsCh = progStarter(newCtx)
			defer progStopper(cancelFunc, wg, statsCh)
		}

		err = dbData.Ddb.PullChunks(ctx, tmpDir, srcDB, toFetch, statsCh, skipCmts)
		if err == pull.ErrDBUpToDate {
			err = nil
		}
		return err
	}()
	if err != nil {
		return err
	}

	for _, newHead := range newHeads {
		remoteTrackRef := newHead.Ref

		// Handle tag references differently from commit references
		if remoteTrackRef.GetType() == ref.TagRefType {
			// For tag references, use SetHead directly with the tag hash
			err := dbData.Ddb.SetHead(ctx, remoteTrackRef, newHead.Hash)
			if err != nil {
				return err
			}
		} else {
			// For commit references, use the existing logic
			optCmt, err := dbData.Ddb.ReadCommit(ctx, newHead.Hash)
			if err != nil {
				return err
			}
			commit, ok := optCmt.ToCommit()
			if !ok {
				// Dest DB should have each hash in `newHeads` now. If we can't read a commit, something is wrong.
				return doltdb.ErrGhostCommitRuntimeFailure
			}

			if mode.Force {
				// TODO: can't be used safely in a SQL context
				err := dbData.Ddb.SetHeadToCommit(ctx, remoteTrackRef, commit)
				if err != nil {
					return err
				}
			} else {
				ok, err := dbData.Ddb.CanFastForward(ctx, remoteTrackRef, commit)
				if err != nil && !errors.Is(err, doltdb.ErrUpToDate) {
					return fmt.Errorf("%w: %s", ErrCantFF, err.Error())
				}
				if !ok {
					return ErrCantFF
				}


Review comment (suggestion):
(this comment won't really make sense in the future after this PR is merged in)
```suggestion
```

Code after:
			wg, statsCh = progStarter(newCtx)
			defer progStopper(cancelFunc, wg, statsCh)
		}

		err = dbData.Ddb.PullChunks(ctx, tmpDir, srcDB, toFetch, statsCh, skipCmts)
		if err == pull.ErrDBUpToDate {
			err = nil
		}
		return err
	}()
	if err != nil {
		return err
	}

	for _, newHead := range newHeads {
		remoteTrackRef := newHead.Ref

		// Handle tag references differently from commit references
		if remoteTrackRef.GetType() == ref.TagRefType {
			// For tag references, use SetHead directly with the tag hash
			err := dbData.Ddb.SetHead(ctx, remoteTrackRef, newHead.Hash)
			if err != nil {
				return err
			}
		} else {
			optCmt, err := dbData.Ddb.ReadCommit(ctx, newHead.Hash)
			if err != nil {
				return err
			}
			commit, ok := optCmt.ToCommit()
			if !ok {
				// Dest DB should have each hash in `newHeads` now. If we can't read a commit, something is wrong.
				return doltdb.ErrGhostCommitRuntimeFailure
			}

			if mode.Force {
				// TODO: can't be used safely in a SQL context
				err := dbData.Ddb.SetHeadToCommit(ctx, remoteTrackRef, commit)
				if err != nil {
					return err
				}
			} else {
				ok, err := dbData.Ddb.CanFastForward(ctx, remoteTrackRef, commit)
				if err != nil && !errors.Is(err, doltdb.ErrUpToDate) {
					return fmt.Errorf("%w: %s", ErrCantFF, err.Error())
				}
				if !ok {
					return ErrCantFF
				}

				switch err {