From b8bdff3c50ad77aa563dcda0f5b9775db779a1e7 Mon Sep 17 00:00:00 2001 From: Andy Arthur Date: Wed, 1 Apr 2020 19:26:12 -0700 Subject: [PATCH] fix nits --- go/libraries/doltcore/doltdb/root_val.go | 1 - .../doltcore/envtestutils/unique_tags_test.go | 48 ------------------- go/libraries/doltcore/schema/tag.go | 7 +-- 3 files changed, 4 insertions(+), 52 deletions(-) diff --git a/go/libraries/doltcore/doltdb/root_val.go b/go/libraries/doltcore/doltdb/root_val.go index bd8cb1bcbc..a37a95566f 100644 --- a/go/libraries/doltcore/doltdb/root_val.go +++ b/go/libraries/doltcore/doltdb/root_val.go @@ -276,7 +276,6 @@ func (root *RootValue) GenerateTagsForNewColumns(ctx context.Context, tableName }) } - newTags := make([]uint64, len(newColNames)) for i := range newTags { newTags[i] = schema.AutoGenerateTag(rootSuperSchema, tableName, existingColKinds, newColNames[i], newColKinds[i]) diff --git a/go/libraries/doltcore/envtestutils/unique_tags_test.go b/go/libraries/doltcore/envtestutils/unique_tags_test.go index 6474b4f43a..4a5dbe5428 100644 --- a/go/libraries/doltcore/envtestutils/unique_tags_test.go +++ b/go/libraries/doltcore/envtestutils/unique_tags_test.go @@ -140,54 +140,6 @@ var UniqueTagsTests = []UniqueTagsTest{ ExpectedBranch: "master", ExpectedErrStr: "two different columns with the same tag", }, - //{ - // Name: "tag conflicts will be rebased on merge", - // Commands: []tc.Command{ - // tc.Query{Query: `create table test (pk int not null primary key comment 'tag:0');`}, - // tc.CommitAll{Message: "created table test"}, - // tc.Branch{BranchName: "other"}, - // tc.Checkout{BranchName: "other"}, - // tc.Query{Query: `alter table test add column c1 int comment 'tag:42';`}, - // tc.CommitAll{Message: "added column c1 with tag 42 to test on branch other"}, - // tc.Checkout{BranchName: "master"}, - // tc.Query{Query: `alter table test add column c0 int comment 'tag:42';`}, - // tc.CommitAll{Message: "added column c0 with tag 42 to test on branch master"}, - // tc.Merge{BranchName: "other"}, - // //tc.Query{Query: `alter table test add column c1 int comment 'tag:42';`}, - // }, - // TableName: "test", - // ExpectedSchema: schema.SchemaFromCols(columnCollection( - // newColTypeInfo("pk", 0, typeinfo.Int32Type, true, schema.NotNullConstraint{}), - // newColTypeInfo("c0", 42, typeinfo.Int32Type, false), - // newColTypeInfo("c1", 10707, typeinfo.Int32Type, false), - // )), - // ExpectedBranch: "master", - //}, - //{ - // Name: "tag conflicts in multiple tables will be rebased on merge", - // Commands: []tc.Command{ - // tc.Query{Query: `create table one (pk1 int not null primary key comment 'tag:1');`}, - // tc.Query{Query: `create table two (pk2 int not null primary key comment 'tag:2');`}, - // tc.CommitAll{Message: "created two tables on master"}, - // tc.Branch{BranchName: "myBranch"}, - // tc.Query{Query: `alter table one add column c1 float comment 'tag:142';`}, - // tc.Query{Query: `alter table two add column c2 float comment 'tag:242';`}, - // tc.CommitAll{Message: "added columns to both tables on master"}, - // tc.Checkout{BranchName: "myBranch"}, - // tc.Query{Query: `alter table one add column c11 float comment 'tag:142';`}, - // tc.Query{Query: `alter table two add column c22 float comment 'tag:242';`}, - // tc.CommitAll{Message: "added columns to both tables on myBranch"}, - // tc.Checkout{BranchName: "master"}, - // tc.Merge{BranchName: "myBranch"}, - // }, - // TableName: "two", - // ExpectedSchema: schema.SchemaFromCols(columnCollection( - // newColTypeInfo("pk2", 2, typeinfo.Int32Type, true, schema.NotNullConstraint{}), - // newColTypeInfo("c2", 242, typeinfo.Float32Type, false), - // newColTypeInfo("c22", 310, typeinfo.Float32Type, false), - // )), - // ExpectedBranch: "master", - //}, } func TestUniqueTags(t *testing.T) { diff --git a/go/libraries/doltcore/schema/tag.go b/go/libraries/doltcore/schema/tag.go index 61a66f8b44..8cb630ec57 100644 --- a/go/libraries/doltcore/schema/tag.go +++ b/go/libraries/doltcore/schema/tag.go @@ -32,7 +32,7 @@ const ( // AutoGenerateTag generates a random tag that doesn't exist in the provided SuperSchema. // It uses a deterministic random number generator that is seeded with the NomsKinds of any existing columns in the // schema and the NomsKind of the column being added to the schema. Deterministic tag generation means that branches -// and repositories that perform the same sequence of mutations to a database will get equivalent databased as a result. +// and repositories that perform the same sequence of mutations to a database will get equivalent databases as a result. // DETERMINISTIC MUTATION IS A CRITICAL INVARIANT TO MAINTAINING COMPATIBILITY BETWEEN REPOSITORIES. // DO NOT ALTER THIS METHOD. func AutoGenerateTag(rootSS *SuperSchema, tableName string, existingColKinds []types.NomsKind, newColName string, newColKind types.NomsKind) uint64 { @@ -40,10 +40,11 @@ func AutoGenerateTag(rootSS *SuperSchema, tableName string, existingColKinds []t var maxTagVal uint64 = 128 * 128 for maxTagVal/2 < uint64(rootSS.Size()) { - if maxTagVal == ReservedTagMin-1 { + if maxTagVal >= ReservedTagMin-1 { panic("There is no way anyone should ever have this many columns. You are a bad person if you hit this panic.") } else if maxTagVal*128 < maxTagVal { maxTagVal = ReservedTagMin - 1 + break } else { maxTagVal = maxTagVal * 128 } @@ -65,7 +66,7 @@ func AutoGenerateTag(rootSS *SuperSchema, tableName string, existingColKinds []t // randomTagGeneratorFromKinds creates a deterministic random number generator that is seeded with the NomsKinds of any // existing columns in the schema and the NomsKind of the column being added to the schema. Deterministic tag generation // means that branches and repositories that perform the same sequence of mutations to a database will get equivalent -// databased as a result. +// databases as a result. // DETERMINISTIC MUTATION IS A CRITICAL INVARIANT TO MAINTAINING COMPATIBILITY BETWEEN REPOSITORIES. // DO NOT ALTER THIS METHOD. func deterministicRandomTagGenerator(tableName string, newColName string, existingColKinds []types.NomsKind, newColKind types.NomsKind) *rand.Rand {