From 87d1540846af4b9b7321765184e83ebea598470e Mon Sep 17 00:00:00 2001 From: James Cor Date: Fri, 5 Apr 2024 10:42:27 -0700 Subject: [PATCH 01/16] init --- .../doltcore/doltdb/foreign_key_coll.go | 14 +++---- .../sqle/enginetest/dolt_engine_test.go | 41 ++++++------------- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 3a733dd929a..0934f3bd9a1 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -361,13 +361,13 @@ func (fkc *ForeignKeyCollection) AddKeys(fks ...ForeignKey) error { if len(key.TableColumns) != len(key.ReferencedTableColumns) { return fmt.Errorf("foreign keys must have the same number of columns declared and referenced") } - if key.IsResolved() { - if _, ok := fkc.GetByTags(key.TableColumns, key.ReferencedTableColumns); ok { - // this differs from MySQL's logic - return fmt.Errorf("a foreign key over columns %v and referenced columns %v already exists", - key.TableColumns, key.ReferencedTableColumns) - } - } + //if key.IsResolved() { + // if _, ok := fkc.GetByTags(key.TableColumns, key.ReferencedTableColumns); ok { + // // this differs from MySQL's logic + // return fmt.Errorf("a foreign key over columns %v and referenced columns %v already exists", + // key.TableColumns, key.ReferencedTableColumns) + // } + //} fkHash, err := key.HashOf() if err != nil { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 2d07b4b36f8..2b453f21fed 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -141,50 +141,33 @@ func TestSchemaOverrides(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - t.Skip() - var scripts = []queries.ScriptTest{ { - Name: "physical columns added after virtual one", + Name: "delete me", SetUpScript: []string{ - "create table t (pk int primary key, col1 int as (pk + 1));", - "insert into t (pk) values (1), (3)", - "alter table t add index idx1 (col1, pk);", - "alter table t add index idx2 (col1);", - "alter table t add column col2 int;", - "alter table t add column col3 int;", - "insert into t (pk, col2, col3) values (2, 4, 5);", + "create table t (i int, index(i));", + "alter table t add index (i)", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "select * from t where col1 = 2", + Query: "show create table t", Expected: []sql.Row{ - {1, 2, nil, nil}, }, }, }, }, } - tcc := &testCommitClock{} - cleanup := installTestCommitClock(tcc) - defer cleanup() - for _, script := range scripts { - sql.RunWithNowFunc(tcc.Now, func() error { - harness := newDoltHarness(t) - harness.Setup(setup.MydbData) - - engine, err := harness.NewEngine(t) - if err != nil { - panic(err) - } - // engine.EngineAnalyzer().Debug = true - // engine.EngineAnalyzer().Verbose = true + harness := newDoltHarness(t) + engine, err := harness.NewEngine(t) + if err != nil { + panic(err) + } + // engine.EngineAnalyzer().Debug = true + // engine.EngineAnalyzer().Verbose = true - enginetest.TestScriptWithEngine(t, engine, harness, script) - return nil - }) + enginetest.TestScriptWithEngine(t, engine, harness, script) } } From fddb204b9abe0a1292885aebf2f1dc58c81feb13 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 10:24:15 -0700 Subject: [PATCH 02/16] adding error --- go/libraries/doltcore/doltdb/foreign_key_coll.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 0934f3bd9a1..fdfb2d69710 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -28,6 +28,8 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/set" "github.com/dolthub/dolt/go/store/hash" "github.com/dolthub/dolt/go/store/types" + + "github.com/dolthub/go-mysql-server/sql" ) // ForeignKeyCollection represents the collection of foreign keys for a root value. @@ -356,7 +358,7 @@ func (fkc *ForeignKeyCollection) AddKeys(fks ...ForeignKey) error { } if _, ok := fkc.GetByNameCaseInsensitive(key.Name); ok { - return fmt.Errorf("a foreign key with the name `%s` already exists", key.Name) + return sql.ErrForeignKeyDuplicateName.New(key.Name) } if len(key.TableColumns) != len(key.ReferencedTableColumns) { return fmt.Errorf("foreign keys must have the same number of columns declared and referenced") From c48d7a7e5952aaad4edc48a1d8feeaa1c6a80fb4 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 10:28:26 -0700 Subject: [PATCH 03/16] bump --- go/go.mod | 2 +- go/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/go.mod b/go/go.mod index f02708e6180..c350f32cb20 100644 --- a/go/go.mod +++ b/go/go.mod @@ -57,7 +57,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.18.1-0.20240405190351-4baf7c65a6db + github.com/dolthub/go-mysql-server v0.18.1-0.20240408081830-3efbe6eb5b6c github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/go-github/v57 v57.0.0 diff --git a/go/go.sum b/go/go.sum index 30e622c3812..e0618025a37 100644 --- a/go/go.sum +++ b/go/go.sum @@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.18.1-0.20240405190351-4baf7c65a6db h1:sVUQthU4rujhbv0DeZ2xcQmGWWbacInalfjVYmb0vSQ= -github.com/dolthub/go-mysql-server v0.18.1-0.20240405190351-4baf7c65a6db/go.mod h1:8zjK76NDWRel1CFdg+DDzy/D5tdOeFOYKBcqf7IB+aA= +github.com/dolthub/go-mysql-server v0.18.1-0.20240408081830-3efbe6eb5b6c h1:p3IG4NO7vslNm030Mx4ky+H186009wv8eHl8h8dl0f4= +github.com/dolthub/go-mysql-server v0.18.1-0.20240408081830-3efbe6eb5b6c/go.mod h1:8zjK76NDWRel1CFdg+DDzy/D5tdOeFOYKBcqf7IB+aA= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ= From e1ef2eb613557de3da2bfff9eb1b592c634ffcca Mon Sep 17 00:00:00 2001 From: jycor Date: Mon, 8 Apr 2024 17:50:02 +0000 Subject: [PATCH 04/16] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 2b453f21fed..f32c020e509 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -150,9 +150,8 @@ func TestSingleScript(t *testing.T) { }, Assertions: []queries.ScriptTestAssertion{ { - Query: "show create table t", - Expected: []sql.Row{ - }, + Query: "show create table t", + Expected: []sql.Row{}, }, }, }, From 30a66a6cf51c6c5ad60bf7ca3b8590aba1831d0a Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 11:28:19 -0700 Subject: [PATCH 05/16] remove hashing for fk names --- go/go.mod | 2 +- go/go.sum | 4 ++-- .../doltcore/doltdb/foreign_key_coll.go | 20 +------------------ 3 files changed, 4 insertions(+), 22 deletions(-) diff --git a/go/go.mod b/go/go.mod index c350f32cb20..7375d12026f 100644 --- a/go/go.mod +++ b/go/go.mod @@ -57,7 +57,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.18.1-0.20240408081830-3efbe6eb5b6c + github.com/dolthub/go-mysql-server v0.18.1-0.20240408181142-d00ec63533a5 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/go-github/v57 v57.0.0 diff --git a/go/go.sum b/go/go.sum index e0618025a37..c4fa040f792 100644 --- a/go/go.sum +++ b/go/go.sum @@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.18.1-0.20240408081830-3efbe6eb5b6c h1:p3IG4NO7vslNm030Mx4ky+H186009wv8eHl8h8dl0f4= -github.com/dolthub/go-mysql-server v0.18.1-0.20240408081830-3efbe6eb5b6c/go.mod h1:8zjK76NDWRel1CFdg+DDzy/D5tdOeFOYKBcqf7IB+aA= +github.com/dolthub/go-mysql-server v0.18.1-0.20240408181142-d00ec63533a5 h1:jaUg8wRY0tZtbH5ekhJRxbIwhRb4xtg1gWkJk9ymY50= +github.com/dolthub/go-mysql-server v0.18.1-0.20240408181142-d00ec63533a5/go.mod h1:8zjK76NDWRel1CFdg+DDzy/D5tdOeFOYKBcqf7IB+aA= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ= diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index fdfb2d69710..d6b49c83b56 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -345,31 +345,12 @@ func NewForeignKeyCollection(keys ...ForeignKey) (*ForeignKeyCollection, error) // both column counts are equal. All other validation should occur before being added to the collection. func (fkc *ForeignKeyCollection) AddKeys(fks ...ForeignKey) error { for _, key := range fks { - if key.Name == "" { - // assign a name based on the hash - // 8 char = 5 base32 bytes, should be collision resistant - // TODO: constraint names should be unique, and this isn't guaranteed to be. - // This logic needs to live at the table / DB level. - fkHash, err := key.HashOf() - if err != nil { - return err - } - key.Name = fkHash.String()[:8] - } - if _, ok := fkc.GetByNameCaseInsensitive(key.Name); ok { return sql.ErrForeignKeyDuplicateName.New(key.Name) } if len(key.TableColumns) != len(key.ReferencedTableColumns) { return fmt.Errorf("foreign keys must have the same number of columns declared and referenced") } - //if key.IsResolved() { - // if _, ok := fkc.GetByTags(key.TableColumns, key.ReferencedTableColumns); ok { - // // this differs from MySQL's logic - // return fmt.Errorf("a foreign key over columns %v and referenced columns %v already exists", - // key.TableColumns, key.ReferencedTableColumns) - // } - //} fkHash, err := key.HashOf() if err != nil { @@ -532,6 +513,7 @@ func (fkc *ForeignKeyCollection) GetMatchingKey(fk ForeignKey, allSchemas map[st OuterLoopResolved: for _, existingFk := range fkc.foreignKeys { if existingFk.IsResolved() { + // TODO: so just fk.Equals(existingFk) would work??? why is this so complicated? // When both are resolved, we do a standard tag comparison if len(fk.TableColumns) != len(existingFk.TableColumns) || len(fk.ReferencedTableColumns) != len(existingFk.ReferencedTableColumns) { From 8f5037e93e4fbdaa3a0052ce120a435136a7cf6d Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 11:33:56 -0700 Subject: [PATCH 06/16] fix bats tests --- integration-tests/bats/diff.bats | 2 +- integration-tests/bats/foreign-keys.bats | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/integration-tests/bats/diff.bats b/integration-tests/bats/diff.bats index c4c9b9b252a..8ec91940b97 100644 --- a/integration-tests/bats/diff.bats +++ b/integration-tests/bats/diff.bats @@ -681,7 +681,7 @@ SQL run dolt diff [ "$status" -eq 0 ] - [[ "$output" =~ "+ CONSTRAINT \`qvvgfpe7\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false + [[ "$output" =~ "+ CONSTRAINT \`child_ibfk_1\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false } @test "diff: new foreign key added without foreign key check, and does not resolve" { diff --git a/integration-tests/bats/foreign-keys.bats b/integration-tests/bats/foreign-keys.bats index 6e6327c1757..b3086b46fc8 100644 --- a/integration-tests/bats/foreign-keys.bats +++ b/integration-tests/bats/foreign-keys.bats @@ -134,17 +134,17 @@ SQL run dolt sql -q "select constraint_name, table_name, column_name, ordinal_position, position_in_unique_constraint, referenced_table_name, referenced_column_name from information_schema.KEY_COLUMN_USAGE;" -r csv [[ "$output" =~ "PRIMARY,colors,id,1,,," ]] || false [[ "$output" =~ "PRIMARY,materials,id,1,,," ]] || false - [[ "$output" =~ "npjt0g99,materials,color,1,1,colors,color" ]] || false + [[ "$output" =~ "materials_ibfk_1,materials,color,1,1,colors,color" ]] || false [[ "$output" =~ "PRIMARY,objects,id,1,,," ]] || false - [[ "$output" =~ "ootftvit,objects,color,1,1,materials,color" ]] || false - [[ "$output" =~ "ootftvit,objects,material,2,2,materials,material" ]] || false + [[ "$output" =~ "objects_ibfk_1,objects,color,1,1,materials,color" ]] || false + [[ "$output" =~ "objects_ibfk_1,objects,material,2,2,materials,material" ]] || false [[ "$output" =~ "PRIMARY,child,id,1,,," ]] || false [[ "$output" =~ "PRIMARY,parent,id,1,,," ]] || false # check information_schema.TABLE_CONSTRAINTS table run dolt sql -q "select * from information_schema.TABLE_CONSTRAINTS where table_name = 'materials';" -r csv [[ "$output" =~ "def,dolt-repo-$$,PRIMARY,dolt-repo-$$,materials,PRIMARY KEY,YES" ]] || false - [[ "$output" =~ "def,dolt-repo-$$,npjt0g99,dolt-repo-$$,materials,FOREIGN KEY,YES" ]] || false + [[ "$output" =~ "def,dolt-repo-$$,materials_ibfk_1,dolt-repo-$$,materials,FOREIGN KEY,YES" ]] || false # check information_schema.TABLE_CONSTRAINTS_EXTENSIONS table run dolt sql -q "select constraint_name from information_schema.TABLE_CONSTRAINTS_EXTENSIONS where table_name = 'materials';" -r csv From ca6e93401aa6bd9e7f278117d287bfcbf4d2eb5a Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 11:39:10 -0700 Subject: [PATCH 07/16] fix unit tests --- go/libraries/doltcore/doltdb/foreign_key_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_test.go b/go/libraries/doltcore/doltdb/foreign_key_test.go index a51630de127..10667ec9392 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_test.go +++ b/go/libraries/doltcore/doltdb/foreign_key_test.go @@ -504,7 +504,7 @@ var foreignKeyTests = []foreignKeyTest{ }, fks: []doltdb.ForeignKey{ { - Name: "1ncba7pr", + Name: "child_ibfk_1", TableName: "child", TableIndex: "v1_idx", TableColumns: []uint64{1215}, @@ -529,7 +529,7 @@ var foreignKeyTests = []foreignKeyTest{ }, fks: []doltdb.ForeignKey{ { - Name: "7nt5f9b0", + Name: "new_table_ibfk_1", TableName: "new_table", // unnamed indexes take the column name TableIndex: "v1", @@ -555,7 +555,7 @@ var foreignKeyTests = []foreignKeyTest{ }, fks: []doltdb.ForeignKey{ { - Name: "k48mbatd", + Name: "child_ibfk_1", TableName: "child", TableIndex: "v1v2_idx", TableColumns: []uint64{1215, 8734}, @@ -581,7 +581,7 @@ var foreignKeyTests = []foreignKeyTest{ }, fks: []doltdb.ForeignKey{ { - Name: "1ncba7pr", + Name: "child_ibfk_1", TableName: "child", TableIndex: "v1_idx", TableColumns: []uint64{1215}, @@ -594,7 +594,7 @@ var foreignKeyTests = []foreignKeyTest{ }, }, { - Name: "8geddp18", + Name: "child_ibfk_2", TableName: "child", TableIndex: "v2_idx", TableColumns: []uint64{8734}, From c05dfe7e809b87bc1eb9a099ed55bd9e157b192c Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 11:42:44 -0700 Subject: [PATCH 08/16] revert test --- .../sqle/enginetest/dolt_engine_test.go | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index f32c020e509..2d07b4b36f8 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -141,32 +141,50 @@ func TestSchemaOverrides(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { + t.Skip() + var scripts = []queries.ScriptTest{ { - Name: "delete me", + Name: "physical columns added after virtual one", SetUpScript: []string{ - "create table t (i int, index(i));", - "alter table t add index (i)", + "create table t (pk int primary key, col1 int as (pk + 1));", + "insert into t (pk) values (1), (3)", + "alter table t add index idx1 (col1, pk);", + "alter table t add index idx2 (col1);", + "alter table t add column col2 int;", + "alter table t add column col3 int;", + "insert into t (pk, col2, col3) values (2, 4, 5);", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "show create table t", - Expected: []sql.Row{}, + Query: "select * from t where col1 = 2", + Expected: []sql.Row{ + {1, 2, nil, nil}, + }, }, }, }, } + tcc := &testCommitClock{} + cleanup := installTestCommitClock(tcc) + defer cleanup() + for _, script := range scripts { - harness := newDoltHarness(t) - engine, err := harness.NewEngine(t) - if err != nil { - panic(err) - } - // engine.EngineAnalyzer().Debug = true - // engine.EngineAnalyzer().Verbose = true + sql.RunWithNowFunc(tcc.Now, func() error { + harness := newDoltHarness(t) + harness.Setup(setup.MydbData) + + engine, err := harness.NewEngine(t) + if err != nil { + panic(err) + } + // engine.EngineAnalyzer().Debug = true + // engine.EngineAnalyzer().Verbose = true - enginetest.TestScriptWithEngine(t, engine, harness, script) + enginetest.TestScriptWithEngine(t, engine, harness, script) + return nil + }) } } From c50dc0d75b995db500c22a3518a7b0efb9c85992 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 11:43:57 -0700 Subject: [PATCH 09/16] remove todo --- go/libraries/doltcore/doltdb/foreign_key_coll.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index d6b49c83b56..51e52eb681f 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -513,7 +513,6 @@ func (fkc *ForeignKeyCollection) GetMatchingKey(fk ForeignKey, allSchemas map[st OuterLoopResolved: for _, existingFk := range fkc.foreignKeys { if existingFk.IsResolved() { - // TODO: so just fk.Equals(existingFk) would work??? why is this so complicated? // When both are resolved, we do a standard tag comparison if len(fk.TableColumns) != len(existingFk.TableColumns) || len(fk.ReferencedTableColumns) != len(existingFk.ReferencedTableColumns) { From 0bb77684ac4c1855cb7cddffff47ffa78359d17b Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 11:53:22 -0700 Subject: [PATCH 10/16] bump --- go/go.mod | 2 +- go/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/go.mod b/go/go.mod index 7375d12026f..4a5e914789f 100644 --- a/go/go.mod +++ b/go/go.mod @@ -57,7 +57,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.18.1-0.20240408181142-d00ec63533a5 + github.com/dolthub/go-mysql-server v0.18.1-0.20240408184140-0d3fca5d9b08 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/go-github/v57 v57.0.0 diff --git a/go/go.sum b/go/go.sum index c4fa040f792..611b44e65fe 100644 --- a/go/go.sum +++ b/go/go.sum @@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.18.1-0.20240408181142-d00ec63533a5 h1:jaUg8wRY0tZtbH5ekhJRxbIwhRb4xtg1gWkJk9ymY50= -github.com/dolthub/go-mysql-server v0.18.1-0.20240408181142-d00ec63533a5/go.mod h1:8zjK76NDWRel1CFdg+DDzy/D5tdOeFOYKBcqf7IB+aA= +github.com/dolthub/go-mysql-server v0.18.1-0.20240408184140-0d3fca5d9b08 h1:IV+QrvDNBFB7PkMuKx5aJm7dZca2muCeOVaMznHFs7U= +github.com/dolthub/go-mysql-server v0.18.1-0.20240408184140-0d3fca5d9b08/go.mod h1:8zjK76NDWRel1CFdg+DDzy/D5tdOeFOYKBcqf7IB+aA= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ= From 483858304c67cdf490aa6e120274be91c1de7748 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 12:09:07 -0700 Subject: [PATCH 11/16] fix --- go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 3e2d450f015..3366d24d7d1 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -176,7 +176,7 @@ var MergeScripts = []queries.ScriptTest{ { Query: "SELECT from_root_ish, violation_type, hex(dolt_row_hash), aColumn, bColumn, CAST(violation_info as CHAR) FROM dolt_constraint_violations_aTable;", Expected: []sql.Row{ - {doltCommit, "foreign key", "13F8480978D0556FA9AE6DF5745A7ACA", 2, -1, `{"Columns":["bColumn"],"ForeignKey":"ml92huct","Index":"bColumn","OnDelete":"RESTRICT","OnUpdate":"RESTRICT","ReferencedColumns":["pk"],"ReferencedIndex":"","ReferencedTable":"parent","Table":"aTable"}`}, + {doltCommit, "foreign key", "13F8480978D0556FA9AE6DF5745A7ACA", 2, -1, `{"Columns":["bColumn"],"ForeignKey":"atable_ibfk_1","Index":"bColumn","OnDelete":"RESTRICT","OnUpdate":"RESTRICT","ReferencedColumns":["pk"],"ReferencedIndex":"","ReferencedTable":"parent","Table":"aTable"}`}, }, }, { @@ -3474,7 +3474,7 @@ var MergeArtifactsScripts = []queries.ScriptTest{ Assertions: []queries.ScriptTestAssertion{ { Query: "CALL DOLT_MERGE('right');", - ExpectedErrStr: "error storing constraint violation for primary key (( 1 )): another violation already exists\nnew violation: {\"Columns\":[\"col2\"],\"ForeignKey\":\"r8l98srn\",\"Index\":\"col2\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col2\"],\"ReferencedIndex\":\"par_col2_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"} old violation: ({\"Columns\":[\"col1\"],\"ForeignKey\":\"ut564qa1\",\"Index\":\"col1\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col1\"],\"ReferencedIndex\":\"par_col1_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"})", + ExpectedErrStr: "error storing constraint violation for primary key (( 1 )): another violation already exists\nnew violation: {\"Columns\":[\"col2\"],\"ForeignKey\":\"child_ibfk_1\",\"Index\":\"col2\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col2\"],\"ReferencedIndex\":\"par_col2_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"} old violation: ({\"Columns\":[\"col1\"],\"ForeignKey\":\"ut564qa1\",\"Index\":\"col1\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col1\"],\"ReferencedIndex\":\"par_col1_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"})", }, { Query: "SELECT * from parent;", From 118158ca13cfe1cff55165b0b1d0538f229d4a2d Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 8 Apr 2024 12:36:20 -0700 Subject: [PATCH 12/16] more fix --- go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go | 2 +- .../doltcore/sqle/enginetest/dolt_transaction_queries.go | 4 ++-- .../mysql-client-tests/node/workbenchTests/diffs.js | 4 ++-- .../mysql-client-tests/node/workbenchTests/table.js | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 3366d24d7d1..fcf96dd709f 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -3474,7 +3474,7 @@ var MergeArtifactsScripts = []queries.ScriptTest{ Assertions: []queries.ScriptTestAssertion{ { Query: "CALL DOLT_MERGE('right');", - ExpectedErrStr: "error storing constraint violation for primary key (( 1 )): another violation already exists\nnew violation: {\"Columns\":[\"col2\"],\"ForeignKey\":\"child_ibfk_1\",\"Index\":\"col2\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col2\"],\"ReferencedIndex\":\"par_col2_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"} old violation: ({\"Columns\":[\"col1\"],\"ForeignKey\":\"ut564qa1\",\"Index\":\"col1\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col1\"],\"ReferencedIndex\":\"par_col1_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"})", + ExpectedErrStr: "error storing constraint violation for primary key (( 1 )): another violation already exists\nnew violation: {\"Columns\":[\"col1\"],\"ForeignKey\":\"child_ibfk_1\",\"Index\":\"col1\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col1\"],\"ReferencedIndex\":\"par_col1_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"} old violation: ({\"Columns\":[\"col2\"],\"ForeignKey\":\"child_ibfk_2\",\"Index\":\"col2\",\"OnDelete\":\"RESTRICT\",\"OnUpdate\":\"RESTRICT\",\"ReferencedColumns\":[\"col2\"],\"ReferencedIndex\":\"par_col2_idx\",\"ReferencedTable\":\"parent\",\"Table\":\"child\"})", }, { Query: "SELECT * from parent;", diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go index 080dda61f83..8d7507dc70a 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_transaction_queries.go @@ -1998,7 +1998,7 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ "To allow transactions to be committed with constraint violations from a merge or transaction sequencing set @@dolt_force_transaction_commit=1.\n" + "Constraint violations: \n" + "Type: Foreign Key Constraint Violation\n" + - "\tForeignKey: r4cf97vs,\n" + + "\tForeignKey: child_ibfk_1,\n" + "\tTable: child,\n" + "\tReferencedTable: ,\n" + "\tIndex: parent_fk,\n" + @@ -2006,7 +2006,7 @@ var DoltConstraintViolationTransactionTests = []queries.TransactionTest{ }, { Query: "/* client b */ INSERT INTO child VALUES (1, 1);", - ExpectedErrStr: "cannot add or update a child row - Foreign key violation on fk: `r4cf97vs`, table: `child`, referenced table: `parent`, key: `[1]`", + ExpectedErrStr: "cannot add or update a child row - Foreign key violation on fk: `child_ibfk_1`, table: `child`, referenced table: `parent`, key: `[1]`", }, }, }, diff --git a/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js b/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js index 968674e120e..3fee16e86b0 100644 --- a/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js +++ b/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js @@ -272,7 +272,7 @@ export const diffTests = [ " `test_pk` int,\n" + " PRIMARY KEY (`id`),\n" + " KEY `test_pk` (`test_pk`),\n" + - " CONSTRAINT `lfbtivir` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" + + " CONSTRAINT `test_info_ibfk_1` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;", to_create_statement: "", }, @@ -306,7 +306,7 @@ export const diffTests = [ " `test_pk` int,\n" + " PRIMARY KEY (`id`),\n" + " KEY `test_pk` (`test_pk`),\n" + - " CONSTRAINT `lfbtivir` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" + + " CONSTRAINT `test_info_ibfk_1` FOREIGN KEY (`test_pk`) REFERENCES `test` (`pk`)\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;", to_create_statement: "", }, diff --git a/integration-tests/mysql-client-tests/node/workbenchTests/table.js b/integration-tests/mysql-client-tests/node/workbenchTests/table.js index ae1ea7c3e88..152f40a3668 100644 --- a/integration-tests/mysql-client-tests/node/workbenchTests/table.js +++ b/integration-tests/mysql-client-tests/node/workbenchTests/table.js @@ -127,7 +127,7 @@ export const tableTests = [ { CONSTRAINT_CATALOG: "def", CONSTRAINT_SCHEMA: `${dbName}/main`, - CONSTRAINT_NAME: "lfbtivir", + CONSTRAINT_NAME: "test_info_ibfk_1", TABLE_CATALOG: "def", TABLE_SCHEMA: `${dbName}/main`, TABLE_NAME: "test_info", From 94d1ffb90ede6a2415a11b8c3ffa03343559ae05 Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 9 Apr 2024 15:00:06 -0700 Subject: [PATCH 13/16] minor bats fix --- integration-tests/bats/diff.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/diff.bats b/integration-tests/bats/diff.bats index 8ec91940b97..b1f734c78b3 100644 --- a/integration-tests/bats/diff.bats +++ b/integration-tests/bats/diff.bats @@ -695,7 +695,7 @@ SQL run dolt diff [ "$status" -eq 0 ] - [[ ! "$output" =~ "+ CONSTRAINT \`npsbmr30\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false + [[ ! "$output" =~ "+ CONSTRAINT \`child_ibfk_1\` FOREIGN KEY (\`j\`) REFERENCES \`parent\` (\`i\`)" ]] || false } @test "diff: existing foreign key that was resolved is deleted" { From 2ba12cfee6187911d965866bbbfca654bf2d6b67 Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 9 Apr 2024 15:17:25 -0700 Subject: [PATCH 14/16] adding bats tests --- integration-tests/bats/foreign-keys.bats | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/integration-tests/bats/foreign-keys.bats b/integration-tests/bats/foreign-keys.bats index b3086b46fc8..749e43788e1 100644 --- a/integration-tests/bats/foreign-keys.bats +++ b/integration-tests/bats/foreign-keys.bats @@ -1289,6 +1289,37 @@ SQL [[ "$output" =~ "child,1" ]] || false } +@test "foreign-keys: different foreign keys with same name is schema conflict1" { + dolt commit -Am "initial commit" + + dolt checkout -b other + dolt sql -q "alter table child add foreign key (id) references parent (v1)" + dolt commit -Am "other" + + dolt checkout main + dolt sql -q "alter table child add foreign key (id) references parent (v2)" + dolt commit -Am "main" + + run dolt merge other + [ "$status" -eq "1" ] + [[ "$output" =~ "duplicate foreign key constraint name" ]] || false +} + +@test "foreign-keys: same foreign keys with same name is ok1" { + dolt commit -Am "initial commit" + + dolt checkout -b other + dolt sql -q "alter table child add foreign key (id) references parent (id)" + dolt commit -Am "other" + + dolt checkout main + dolt sql -q "alter table child add foreign key (id) references parent (id)" + dolt commit -Am "main" + + run dolt merge other + [ "$status" -eq "0" ] +} + @test "foreign-keys: Resolve catches violations" { dolt sql < Date: Tue, 9 Apr 2024 15:27:16 -0700 Subject: [PATCH 15/16] fixing tests --- integration-tests/bats/foreign-keys.bats | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/foreign-keys.bats b/integration-tests/bats/foreign-keys.bats index 749e43788e1..f18810d8290 100644 --- a/integration-tests/bats/foreign-keys.bats +++ b/integration-tests/bats/foreign-keys.bats @@ -1289,7 +1289,7 @@ SQL [[ "$output" =~ "child,1" ]] || false } -@test "foreign-keys: different foreign keys with same name is schema conflict1" { +@test "foreign-keys: different foreign keys with same name is schema conflict" { dolt commit -Am "initial commit" dolt checkout -b other @@ -1303,9 +1303,17 @@ SQL run dolt merge other [ "$status" -eq "1" ] [[ "$output" =~ "duplicate foreign key constraint name" ]] || false + + run dolt sql -q "alter table child rename constraint foreign key child_ibfk_1 to child_ibfk_2" + [ "$status" -eq "0" ] + + dolt commit -Am "rename" + + run dolt merge other + [ "$status" -eq "0" ] } -@test "foreign-keys: same foreign keys with same name is ok1" { +@test "foreign-keys: same foreign keys with same name is ok" { dolt commit -Am "initial commit" dolt checkout -b other From bfd72821cc40e36cf04730b77b06546cf1a78f35 Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 9 Apr 2024 15:48:52 -0700 Subject: [PATCH 16/16] bump --- go/go.mod | 2 +- go/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/go.mod b/go/go.mod index eef05f2e166..d560886b061 100644 --- a/go/go.mod +++ b/go/go.mod @@ -57,7 +57,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.18.2-0.20240409214735-e32a563f76cf + github.com/dolthub/go-mysql-server v0.18.2-0.20240409224550-bfde35b66568 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/go-github/v57 v57.0.0 diff --git a/go/go.sum b/go/go.sum index 7518a80341a..a298b3baf43 100644 --- a/go/go.sum +++ b/go/go.sum @@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.18.2-0.20240409214735-e32a563f76cf h1:QSxfP2O2HqNd48QaYKlGf0reWkYKnqjHzFTymw1+s6I= -github.com/dolthub/go-mysql-server v0.18.2-0.20240409214735-e32a563f76cf/go.mod h1:uTqIti1oPqKEUS9N1zcT43a/QQ8n0PuBdZUMZziBaGU= +github.com/dolthub/go-mysql-server v0.18.2-0.20240409224550-bfde35b66568 h1:H2VdUQThLeXRdfKma4Llh9YjB6Ba76twe9MvHlDj8NM= +github.com/dolthub/go-mysql-server v0.18.2-0.20240409224550-bfde35b66568/go.mod h1:uTqIti1oPqKEUS9N1zcT43a/QQ8n0PuBdZUMZziBaGU= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ=