From edbd1c9367519dc342ee8abb6bf97aa27d07fc5e Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 23 Feb 2026 16:24:23 -0800 Subject: [PATCH 1/9] hardcode HashOf --- .../doltcore/doltdb/foreign_key_coll.go | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 99cf7468446..8f76216ca99 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -181,38 +181,51 @@ func (fk ForeignKey) DeepEquals(other ForeignKey) bool { // HashOf returns the Noms hash of a ForeignKey. func (fk ForeignKey) HashOf() (hash.Hash, error) { - var fields []interface{} - fields = append(fields, fk.Name, fk.TableName, fk.TableIndex) - for _, t := range fk.TableColumns { - fields = append(fields, t) + // TODO: use unsafe methods for string writes? + var bb bytes.Buffer + var err error + _, err = bb.WriteString(fk.Name) + if err != nil { + return hash.Hash{}, err } - fields = append(fields, fk.ReferencedTableName, fk.ReferencedTableIndex) - for _, t := range fk.ReferencedTableColumns { - fields = append(fields, t) + _, err = bb.WriteString(fk.TableName.String()) + if err != nil { + return hash.Hash{}, err } - fields = append(fields, []byte{byte(fk.OnUpdate), byte(fk.OnDelete)}) - for _, col := range fk.UnresolvedFKDetails.TableColumns { - fields = append(fields, col) + _, err = bb.WriteString(fk.TableIndex) + if err != nil { + return hash.Hash{}, err } - for _, col := range fk.UnresolvedFKDetails.ReferencedTableColumns { - fields = append(fields, col) + for _, col := range fk.TableColumns { + err = binary.Write(&bb, binary.LittleEndian, col) + if err != nil { + return hash.Hash{}, err + } } - - var bb bytes.Buffer - for _, field := range fields { - var err error - switch t := field.(type) { - case string: - _, err = bb.Write([]byte(t)) - case []byte: - _, err = bb.Write(t) - case uint64: - err = binary.Write(&bb, binary.LittleEndian, t) - case TableName: - _, err = bb.Write([]byte(t.String())) - default: - return hash.Hash{}, fmt.Errorf("unsupported type %T", t) + _, err = bb.WriteString(fk.ReferencedTableName.String()) + if err != nil { + return hash.Hash{}, err + } + _, err = bb.WriteString(fk.ReferencedTableIndex) + if err != nil { + return hash.Hash{}, err + } + err = bb.WriteByte(byte(fk.OnUpdate)) + if err != nil { + return hash.Hash{}, err + } + err = bb.WriteByte(byte(fk.OnUpdate)) + if err != nil { + return hash.Hash{}, err + } + for _, col := range fk.UnresolvedFKDetails.TableColumns { + _, err = bb.WriteString(col) + if err != nil { + return hash.Hash{}, err } + } + for _, col := range fk.UnresolvedFKDetails.ReferencedTableColumns { + _, err = bb.WriteString(col) if err != nil { return hash.Hash{}, err } From d5f230a2dac67b8f839430bb0d9c8c2e4628852c Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 23 Feb 2026 16:57:59 -0800 Subject: [PATCH 2/9] no buffer --- .../doltcore/doltdb/foreign_key_coll.go | 56 +++++-------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 8f76216ca99..780128b738a 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -181,57 +181,31 @@ func (fk ForeignKey) DeepEquals(other ForeignKey) bool { // HashOf returns the Noms hash of a ForeignKey. func (fk ForeignKey) HashOf() (hash.Hash, error) { - // TODO: use unsafe methods for string writes? - var bb bytes.Buffer + // TODO: use a pool of byte buffers? + // TODO: the order of these shouldn't matter, but maintain it for now + // TODO: unsafe strings + var bb []byte var err error - _, err = bb.WriteString(fk.Name) - if err != nil { - return hash.Hash{}, err - } - _, err = bb.WriteString(fk.TableName.String()) - if err != nil { - return hash.Hash{}, err - } - _, err = bb.WriteString(fk.TableIndex) - if err != nil { - return hash.Hash{}, err - } + bb = append(bb, fk.Name...) + bb = append(bb, fk.TableName.String()...) + bb = append(bb, fk.TableIndex...) for _, col := range fk.TableColumns { - err = binary.Write(&bb, binary.LittleEndian, col) + bb, err = binary.Append(bb, binary.LittleEndian, col) if err != nil { return hash.Hash{}, err } } - _, err = bb.WriteString(fk.ReferencedTableName.String()) - if err != nil { - return hash.Hash{}, err - } - _, err = bb.WriteString(fk.ReferencedTableIndex) - if err != nil { - return hash.Hash{}, err - } - err = bb.WriteByte(byte(fk.OnUpdate)) - if err != nil { - return hash.Hash{}, err - } - err = bb.WriteByte(byte(fk.OnUpdate)) - if err != nil { - return hash.Hash{}, err - } + bb = append(bb, fk.ReferencedTableName.String()...) + bb = append(bb, fk.ReferencedTableIndex...) + bb = append(bb, byte(fk.OnUpdate)) + bb = append(bb, byte(fk.OnDelete)) for _, col := range fk.UnresolvedFKDetails.TableColumns { - _, err = bb.WriteString(col) - if err != nil { - return hash.Hash{}, err - } + bb = append(bb, col...) } for _, col := range fk.UnresolvedFKDetails.ReferencedTableColumns { - _, err = bb.WriteString(col) - if err != nil { - return hash.Hash{}, err - } + bb = append(bb, col...) } - - return hash.Of(bb.Bytes()), nil + return hash.Of(bb), nil } // CombinedHash returns a combined hash value for all foreign keys in the slice provided. From 2a47efe268af24c7246905434445945ba1c303c2 Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 24 Feb 2026 15:02:12 -0800 Subject: [PATCH 3/9] use pointer receivers --- .../doltcore/doltdb/foreign_key_coll.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 780128b738a..aef00815d50 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -69,7 +69,7 @@ type UnresolvedFKDetails struct { // EqualDefs returns whether two foreign keys have the same definition over the same column sets. // It does not compare table names or foreign key names. -func (fk ForeignKey) EqualDefs(other ForeignKey) bool { +func (fk *ForeignKey) EqualDefs(other ForeignKey) bool { if len(fk.TableColumns) != len(other.TableColumns) || len(fk.ReferencedTableColumns) != len(other.ReferencedTableColumns) { return false } @@ -95,7 +95,7 @@ func (fk ForeignKey) EqualDefs(other ForeignKey) bool { // column names to column tags, which is why |fkSchemasByName| and |otherSchemasByName| are passed in. Each of these // is a map of table schemas for |fk| and |other|, where the child table and every parent table referenced in the // foreign key is present in the map. -func (fk ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByName map[TableName]schema.Schema) bool { +func (fk *ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByName map[TableName]schema.Schema) bool { // If both FKs are resolved or unresolved, we can just deeply compare them if fk.IsResolved() == other.IsResolved() { return fk.DeepEquals(other) @@ -118,9 +118,9 @@ func (fk ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByNam var resolvedFK, unresolvedFK ForeignKey var resolvedSchemasByName map[TableName]schema.Schema if fk.IsResolved() { - resolvedFK, unresolvedFK, resolvedSchemasByName = fk, other, fkSchemasByName + resolvedFK, unresolvedFK, resolvedSchemasByName = *fk, other, fkSchemasByName } else { - resolvedFK, unresolvedFK, resolvedSchemasByName = other, fk, otherSchemasByName + resolvedFK, unresolvedFK, resolvedSchemasByName = other, *fk, otherSchemasByName } // Check the columns on the child table @@ -168,7 +168,7 @@ func (fk ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByNam // table names. Note that if one foreign key is resolved and the other is NOT resolved, // then this function will not calculate equality correctly. When comparing a resolved // FK with an unresolved FK, the ForeignKey.Equals() function should be used instead. -func (fk ForeignKey) DeepEquals(other ForeignKey) bool { +func (fk *ForeignKey) DeepEquals(other ForeignKey) bool { if !fk.EqualDefs(other) { return false } @@ -180,7 +180,7 @@ func (fk ForeignKey) DeepEquals(other ForeignKey) bool { } // HashOf returns the Noms hash of a ForeignKey. -func (fk ForeignKey) HashOf() (hash.Hash, error) { +func (fk *ForeignKey) HashOf() (hash.Hash, error) { // TODO: use a pool of byte buffers? // TODO: the order of these shouldn't matter, but maintain it for now // TODO: unsafe strings @@ -228,17 +228,17 @@ func CombinedHash(fks []ForeignKey) (hash.Hash, error) { } // IsSelfReferential returns whether the table declaring the foreign key is also referenced by the foreign key. -func (fk ForeignKey) IsSelfReferential() bool { +func (fk *ForeignKey) IsSelfReferential() bool { return fk.TableName.EqualFold(fk.ReferencedTableName) } // IsResolved returns whether the foreign key has been resolved. -func (fk ForeignKey) IsResolved() bool { +func (fk *ForeignKey) IsResolved() bool { return len(fk.TableColumns) > 0 && len(fk.ReferencedTableColumns) > 0 } // ValidateReferencedTableSchema verifies that the given schema matches the expectation of the referenced table. -func (fk ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error { +func (fk *ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error { // An unresolved foreign key will be validated later, so we don't return an error here. if !fk.IsResolved() { return nil @@ -260,7 +260,7 @@ func (fk ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error { } // ValidateTableSchema verifies that the given schema matches the expectation of the declaring table. -func (fk ForeignKey) ValidateTableSchema(sch schema.Schema) error { +func (fk *ForeignKey) ValidateTableSchema(sch schema.Schema) error { // An unresolved foreign key will be validated later, so we don't return an error here. if !fk.IsResolved() { return nil From b14dfedcb38c60dc2b1b5655c14321979f3d7d93 Mon Sep 17 00:00:00 2001 From: James Cor Date: Tue, 24 Feb 2026 16:52:35 -0800 Subject: [PATCH 4/9] add missing referenced table columns --- go/libraries/doltcore/doltdb/foreign_key_coll.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index aef00815d50..904ba09c7d2 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -197,6 +197,12 @@ func (fk *ForeignKey) HashOf() (hash.Hash, error) { } bb = append(bb, fk.ReferencedTableName.String()...) bb = append(bb, fk.ReferencedTableIndex...) + for _, col := range fk.ReferencedTableColumns { + bb, err = binary.Append(bb, binary.LittleEndian, col) + if err != nil { + return hash.Hash{}, err + } + } bb = append(bb, byte(fk.OnUpdate)) bb = append(bb, byte(fk.OnDelete)) for _, col := range fk.UnresolvedFKDetails.TableColumns { From df61cf94abc4a5d8dfd03eea6d3e13dbab73912d Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 25 Feb 2026 13:23:19 -0800 Subject: [PATCH 5/9] value receiver is better in this case --- .../doltcore/doltdb/foreign_key_coll.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 904ba09c7d2..99a71326668 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -69,7 +69,7 @@ type UnresolvedFKDetails struct { // EqualDefs returns whether two foreign keys have the same definition over the same column sets. // It does not compare table names or foreign key names. -func (fk *ForeignKey) EqualDefs(other ForeignKey) bool { +func (fk ForeignKey) EqualDefs(other ForeignKey) bool { if len(fk.TableColumns) != len(other.TableColumns) || len(fk.ReferencedTableColumns) != len(other.ReferencedTableColumns) { return false } @@ -95,7 +95,7 @@ func (fk *ForeignKey) EqualDefs(other ForeignKey) bool { // column names to column tags, which is why |fkSchemasByName| and |otherSchemasByName| are passed in. Each of these // is a map of table schemas for |fk| and |other|, where the child table and every parent table referenced in the // foreign key is present in the map. -func (fk *ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByName map[TableName]schema.Schema) bool { +func (fk ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByName map[TableName]schema.Schema) bool { // If both FKs are resolved or unresolved, we can just deeply compare them if fk.IsResolved() == other.IsResolved() { return fk.DeepEquals(other) @@ -118,9 +118,9 @@ func (fk *ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByNa var resolvedFK, unresolvedFK ForeignKey var resolvedSchemasByName map[TableName]schema.Schema if fk.IsResolved() { - resolvedFK, unresolvedFK, resolvedSchemasByName = *fk, other, fkSchemasByName + resolvedFK, unresolvedFK, resolvedSchemasByName = fk, other, fkSchemasByName } else { - resolvedFK, unresolvedFK, resolvedSchemasByName = other, *fk, otherSchemasByName + resolvedFK, unresolvedFK, resolvedSchemasByName = other, fk, otherSchemasByName } // Check the columns on the child table @@ -168,7 +168,7 @@ func (fk *ForeignKey) Equals(other ForeignKey, fkSchemasByName, otherSchemasByNa // table names. Note that if one foreign key is resolved and the other is NOT resolved, // then this function will not calculate equality correctly. When comparing a resolved // FK with an unresolved FK, the ForeignKey.Equals() function should be used instead. -func (fk *ForeignKey) DeepEquals(other ForeignKey) bool { +func (fk ForeignKey) DeepEquals(other ForeignKey) bool { if !fk.EqualDefs(other) { return false } @@ -180,7 +180,7 @@ func (fk *ForeignKey) DeepEquals(other ForeignKey) bool { } // HashOf returns the Noms hash of a ForeignKey. -func (fk *ForeignKey) HashOf() (hash.Hash, error) { +func (fk ForeignKey) HashOf() (hash.Hash, error) { // TODO: use a pool of byte buffers? // TODO: the order of these shouldn't matter, but maintain it for now // TODO: unsafe strings @@ -234,17 +234,17 @@ func CombinedHash(fks []ForeignKey) (hash.Hash, error) { } // IsSelfReferential returns whether the table declaring the foreign key is also referenced by the foreign key. -func (fk *ForeignKey) IsSelfReferential() bool { +func (fk ForeignKey) IsSelfReferential() bool { return fk.TableName.EqualFold(fk.ReferencedTableName) } // IsResolved returns whether the foreign key has been resolved. -func (fk *ForeignKey) IsResolved() bool { +func (fk ForeignKey) IsResolved() bool { return len(fk.TableColumns) > 0 && len(fk.ReferencedTableColumns) > 0 } // ValidateReferencedTableSchema verifies that the given schema matches the expectation of the referenced table. -func (fk *ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error { +func (fk ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error { // An unresolved foreign key will be validated later, so we don't return an error here. if !fk.IsResolved() { return nil @@ -266,7 +266,7 @@ func (fk *ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error { } // ValidateTableSchema verifies that the given schema matches the expectation of the declaring table. -func (fk *ForeignKey) ValidateTableSchema(sch schema.Schema) error { +func (fk ForeignKey) ValidateTableSchema(sch schema.Schema) error { // An unresolved foreign key will be validated later, so we don't return an error here. if !fk.IsResolved() { return nil From 7b8b0efcfc1e77ae991965b878247c686bdb1797 Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 25 Feb 2026 13:48:19 -0800 Subject: [PATCH 6/9] use a buffer pool --- .../doltcore/doltdb/foreign_key_coll.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll.go b/go/libraries/doltcore/doltdb/foreign_key_coll.go index 99a71326668..f542131c9cc 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll.go @@ -22,6 +22,7 @@ import ( "fmt" "sort" "strings" + "sync" "github.com/dolthub/dolt/go/libraries/doltcore/schema" "github.com/dolthub/dolt/go/libraries/utils/set" @@ -179,12 +180,20 @@ func (fk ForeignKey) DeepEquals(other ForeignKey) bool { fk.ReferencedTableIndex == other.ReferencedTableIndex } +var fkBufPool = sync.Pool{ + New: func() interface{} { + return make([]byte, 0, 1024) + }, +} + // HashOf returns the Noms hash of a ForeignKey. func (fk ForeignKey) HashOf() (hash.Hash, error) { - // TODO: use a pool of byte buffers? - // TODO: the order of these shouldn't matter, but maintain it for now - // TODO: unsafe strings - var bb []byte + bb := fkBufPool.Get().([]byte) + defer func() { + bb = bb[:0] + fkBufPool.Put(bb) + }() + var err error bb = append(bb, fk.Name...) bb = append(bb, fk.TableName.String()...) From 9c4bcac0a935bbe06682541118120b76e584897b Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 25 Feb 2026 16:09:22 -0800 Subject: [PATCH 7/9] mini benchmark --- .../doltcore/doltdb/foreign_key_coll_test.go | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 go/libraries/doltcore/doltdb/foreign_key_coll_test.go diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll_test.go b/go/libraries/doltcore/doltdb/foreign_key_coll_test.go new file mode 100644 index 00000000000..f35c4ff21f4 --- /dev/null +++ b/go/libraries/doltcore/doltdb/foreign_key_coll_test.go @@ -0,0 +1,32 @@ +package doltdb + +import "testing" + +// BenchmarkForeignKeyHashOf-14 2012511 578.7 ns/op +// BenchmarkForeignKeyHashOf-14 4511584 260.0 ns/op +func BenchmarkForeignKeyHashOf(b *testing.B) { + fk := ForeignKey{ + Name: "name", + TableName: TableName{ + Name: "tbl_name", + Schema: "tbl_schema", + }, + TableIndex: "tbl_index", + TableColumns: []uint64{1, 2, 3}, + ReferencedTableName: TableName{ + Name: "reftbl_name", + Schema: "reftbl_schema", + }, + ReferencedTableIndex: "reftbl_index", + ReferencedTableColumns: []uint64{1, 2, 3}, + OnUpdate: ForeignKeyReferentialAction_Cascade, + OnDelete: ForeignKeyReferentialAction_Cascade, + UnresolvedFKDetails: UnresolvedFKDetails{ + TableColumns: []string{"asdf"}, + ReferencedTableColumns: []string{"asdf"}, + }, + } + for i := 0; i < b.N; i++ { + _, _ = fk.HashOf() + } +} From c0932c50006560730060d1ac703e55ce567eb2d4 Mon Sep 17 00:00:00 2001 From: James Cor Date: Thu, 26 Feb 2026 10:15:28 -0800 Subject: [PATCH 8/9] add header --- .../doltcore/doltdb/foreign_key_coll_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll_test.go b/go/libraries/doltcore/doltdb/foreign_key_coll_test.go index f35c4ff21f4..26603276797 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll_test.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll_test.go @@ -1,3 +1,17 @@ +// Copyright 2026 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 doltdb import "testing" From e5a6ba734873771b6c320cc7c08e3b66f45f050a Mon Sep 17 00:00:00 2001 From: James Cor Date: Thu, 26 Feb 2026 13:10:23 -0800 Subject: [PATCH 9/9] remove comment --- go/libraries/doltcore/doltdb/foreign_key_coll_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/go/libraries/doltcore/doltdb/foreign_key_coll_test.go b/go/libraries/doltcore/doltdb/foreign_key_coll_test.go index 26603276797..f54ad024b0e 100644 --- a/go/libraries/doltcore/doltdb/foreign_key_coll_test.go +++ b/go/libraries/doltcore/doltdb/foreign_key_coll_test.go @@ -16,8 +16,6 @@ package doltdb import "testing" -// BenchmarkForeignKeyHashOf-14 2012511 578.7 ns/op -// BenchmarkForeignKeyHashOf-14 4511584 260.0 ns/op func BenchmarkForeignKeyHashOf(b *testing.B) { fk := ForeignKey{ Name: "name",