From e31d29ba61cdb97bceb9316d72eced347d2e6605 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 12 Apr 2023 12:30:59 -0700 Subject: [PATCH 1/4] Consider remote refs when determining a table's auto increment --- .../globalstate/auto_increment_tracker.go | 17 ++++-- .../doltcore/sqle/globalstate/global_state.go | 56 +++++++++++++------ 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go b/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go index af13fa49b1..473447d83b 100644 --- a/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go +++ b/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go @@ -31,16 +31,23 @@ type AutoIncrementTracker struct { mu *sync.Mutex } -// NewAutoIncrementTracker returns a new autoincrement tracker for the working sets given. All working sets must be +// NewAutoIncrementTracker returns a new autoincrement tracker for the roots given. All roots sets must be // considered because the auto increment value for a table is tracked globally, across all branches. -func NewAutoIncrementTracker(ctx context.Context, wses ...*doltdb.WorkingSet) (AutoIncrementTracker, error) { +// Roots provided should be the working sets when available, or the branches when they are not (e.g. for remote +// branches that don't have a local working set) +func NewAutoIncrementTracker(ctx context.Context, roots ...doltdb.Rootish) (AutoIncrementTracker, error) { ait := AutoIncrementTracker{ sequences: make(map[string]uint64), mu: &sync.Mutex{}, } - - for _, ws := range wses { - err := ws.WorkingRoot().IterTables(ctx, func(tableName string, table *doltdb.Table, sch schema.Schema) (bool, error) { + + for _, ws := range roots { + root, err := ws.ResolveRootValue(ctx) + if err != nil { + return AutoIncrementTracker{}, err + } + + err = root.IterTables(ctx, func(tableName string, table *doltdb.Table, sch schema.Schema) (bool, error) { ok := schema.HasAutoIncrement(sch) if !ok { return false, nil diff --git a/go/libraries/doltcore/sqle/globalstate/global_state.go b/go/libraries/doltcore/sqle/globalstate/global_state.go index 6d59040694..5f7ad36a5a 100644 --- a/go/libraries/doltcore/sqle/globalstate/global_state.go +++ b/go/libraries/doltcore/sqle/globalstate/global_state.go @@ -34,25 +34,47 @@ func NewGlobalStateStoreForDb(ctx context.Context, db *doltdb.DoltDB) (GlobalSta return GlobalState{}, err } - var wses []*doltdb.WorkingSet - for _, b := range branches { - wsRef, err := ref.WorkingSetRefForHead(b) - if err != nil { - return GlobalState{}, err - } - - ws, err := db.ResolveWorkingSet(ctx, wsRef) - if err == doltdb.ErrWorkingSetNotFound { - // skip, continue working on other branches - continue - } else if err != nil { - return GlobalState{}, err - } - - wses = append(wses, ws) + remotes, err := db.GetRemoteRefs(ctx) + if err != nil { + return GlobalState{}, err } - tracker, err := NewAutoIncrementTracker(ctx, wses...) + rootRefs := make([]ref.DoltRef, 0, len(branches) + len(remotes)) + rootRefs = append(rootRefs, branches...) + rootRefs = append(rootRefs, remotes...) + + var roots []doltdb.Rootish + for _, b := range rootRefs { + switch b.GetType() { + case ref.BranchRefType: + wsRef, err := ref.WorkingSetRefForHead(b) + if err != nil { + return GlobalState{}, err + } + + ws, err := db.ResolveWorkingSet(ctx, wsRef) + if err == doltdb.ErrWorkingSetNotFound { + // use the branch head if there isn't a working set for it + cm, err := db.ResolveCommitRef(ctx, b) + if err != nil { + return GlobalState{}, err + } + roots = append(roots, cm) + } else if err != nil { + return GlobalState{}, err + } else { + roots = append(roots, ws) + } + case ref.RemoteRefType: + cm, err := db.ResolveCommitRef(ctx, b) + if err != nil { + return GlobalState{}, err + } + roots = append(roots, cm) + } + } + + tracker, err := NewAutoIncrementTracker(ctx, roots...) if err != nil { return GlobalState{}, err } From 62592b43848829a0f2b9addd3fb8ca3e76999680 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 12 Apr 2023 12:53:50 -0700 Subject: [PATCH 2/4] Test for auto increment on a newly cloned database --- integration-tests/bats/auto_increment.bats | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/integration-tests/bats/auto_increment.bats b/integration-tests/bats/auto_increment.bats index 5755d3bce5..68e898602f 100644 --- a/integration-tests/bats/auto_increment.bats +++ b/integration-tests/bats/auto_increment.bats @@ -776,3 +776,43 @@ SQL [[ "$output" =~ "5,5" ]] || false [[ "$output" =~ "6,6" ]] || false } + +@test "auto_increment: newly cloned database" { + dolt sql < Date: Wed, 12 Apr 2023 19:59:42 +0000 Subject: [PATCH 3/4] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- .../doltcore/sqle/globalstate/auto_increment_tracker.go | 8 ++++---- go/libraries/doltcore/sqle/globalstate/global_state.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go b/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go index 473447d83b..b344ca0a75 100644 --- a/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go +++ b/go/libraries/doltcore/sqle/globalstate/auto_increment_tracker.go @@ -33,20 +33,20 @@ type AutoIncrementTracker struct { // NewAutoIncrementTracker returns a new autoincrement tracker for the roots given. All roots sets must be // considered because the auto increment value for a table is tracked globally, across all branches. -// Roots provided should be the working sets when available, or the branches when they are not (e.g. for remote -// branches that don't have a local working set) +// Roots provided should be the working sets when available, or the branches when they are not (e.g. for remote +// branches that don't have a local working set) func NewAutoIncrementTracker(ctx context.Context, roots ...doltdb.Rootish) (AutoIncrementTracker, error) { ait := AutoIncrementTracker{ sequences: make(map[string]uint64), mu: &sync.Mutex{}, } - + for _, ws := range roots { root, err := ws.ResolveRootValue(ctx) if err != nil { return AutoIncrementTracker{}, err } - + err = root.IterTables(ctx, func(tableName string, table *doltdb.Table, sch schema.Schema) (bool, error) { ok := schema.HasAutoIncrement(sch) if !ok { diff --git a/go/libraries/doltcore/sqle/globalstate/global_state.go b/go/libraries/doltcore/sqle/globalstate/global_state.go index 5f7ad36a5a..bdeb0219e8 100644 --- a/go/libraries/doltcore/sqle/globalstate/global_state.go +++ b/go/libraries/doltcore/sqle/globalstate/global_state.go @@ -39,7 +39,7 @@ func NewGlobalStateStoreForDb(ctx context.Context, db *doltdb.DoltDB) (GlobalSta return GlobalState{}, err } - rootRefs := make([]ref.DoltRef, 0, len(branches) + len(remotes)) + rootRefs := make([]ref.DoltRef, 0, len(branches)+len(remotes)) rootRefs = append(rootRefs, branches...) rootRefs = append(rootRefs, remotes...) From ee1d330ca743086ab367571b196130f5ccefed52 Mon Sep 17 00:00:00 2001 From: Andy Arthur Date: Thu, 13 Apr 2023 12:17:43 -0700 Subject: [PATCH 4/4] integration-test/bats: fix tests failing due to remote refs being pulled into SQL context --- integration-tests/bats/feature-version.bats | 2 +- integration-tests/bats/sql-server-remotesrv.bats | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/integration-tests/bats/feature-version.bats b/integration-tests/bats/feature-version.bats index 8407ce86ba..610e42e94c 100644 --- a/integration-tests/bats/feature-version.bats +++ b/integration-tests/bats/feature-version.bats @@ -125,7 +125,7 @@ SQL run dolt --feature-version $OLD pull [ "$status" -ne 0 ] [[ "$output" =~ "visit https://github.com/dolthub/dolt/releases/latest/" ]] || false - dolt --feature-version $OLD sql -q "SELECT * FROM test" + dolt --feature-version $OLD ls -v dolt --feature-version $OLD checkout other popd } diff --git a/integration-tests/bats/sql-server-remotesrv.bats b/integration-tests/bats/sql-server-remotesrv.bats index 7db00bbba8..6e10ef9f23 100644 --- a/integration-tests/bats/sql-server-remotesrv.bats +++ b/integration-tests/bats/sql-server-remotesrv.bats @@ -33,6 +33,7 @@ teardown() { dolt sql-server --remotesapi-port 50051 & srv_pid=$! + sleep 2 cd ../ dolt clone http://localhost:50051/remote repo1