Repository: go-gorm/gorm
Pull request #5045 (2022-01-31 20:49:51)

Fix invalid foreign key constraint name with long schema qualified tables
- [x] Do only one thing
- [x] Non breaking API changes
- [x] Tested

### What did this pull request do?

This PR generates syntactically valid names for foreign key constraints when the table name is greater then 64 characters and the table name is schema-qualified, e.g. `my_schema.a_very_very_very_very_very_very_very_very_long_table_name`. 

Fixes https://github.com/go-gorm/gorm/issues/5029.

Previously the name formatter would leave `.` characters in the long schema-qualified table name intact. However, this generates an invalid foreign key constraint in Postgres and migrations fail with an error like this:

```
2022/01/26 17:14:17 Failed to auto migrate, but got error ERROR: syntax error at or near "." (SQLSTATE 42601)
```

The new behavior is for the `.` in the table name to be replaced with `_`.

The `.` to `_` translation was already occurring when the table name was less than 65 characters due to [this code](https://github.com/go-gorm/gorm/blob/master/schema/naming.go#L93).  However, when the table name length hit the 65 character limit the table name was used without any invalid character replacement. One question I have is whether all names that pass through `NamingStrategy.formatName` should have this fix applied. Right now I've only fixed FK names to minimize any unexpected behavior changes.

### User Case Description

This change is necessary for me because I'm working in a database with many schemas and foreign key constraints that span schemas. When a table name is long in this setup it currently causes migrations to fail due to invalid foreign key constraint naming.


jinzhu: Hi @mnussbaum 

I think we should fix the issue in https://github.com/go-gorm/gorm/blob/5edc78116fe46a7d001db52d80a78f97756ac1ad/schema/naming.go#L98

Change it to `formattedName = formattedName[0:56] + hex.EncodeToString(bs)[:8]`?
daheige: @jinzhu I agree what you said this suggestion.


------------------ Original ------------------
From: Jinzhu ***@***.***&gt;
Date: Sun,Feb 20,2022 8:30 AM
To: go-gorm/gorm ***@***.***&gt;
Cc: heige ***@***.***&gt;, Comment ***@***.***&gt;
Subject: Re: [go-gorm/gorm] Fix invalid foreign key constraint name with longschema qualified tables (PR #5045)





 
Hi @mnussbaum
 
I think we should fix the issue in https://github.com/go-gorm/gorm/blob/5edc78116fe46a7d001db52d80a78f97756ac1ad/schema/naming.go#L98
 
Change it to formattedName = formattedName[0:56] + hex.EncodeToString(bs)[:8]?
 
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android. 
You are receiving this because you commented.Message ID: ***@***.***&gt;
mnussbaum: @jinzhu I've pushed an update using your suggestion, looks like that should work to me

Patch:
diff --git a/schema/naming.go b/schema/naming.go
index 125094b..47a2b36 100644
--- a/schema/naming.go
+++ b/schema/naming.go
@@ -3,7 +3,6 @@ package schema
 import (
 	"crypto/sha1"
 	"encoding/hex"
-	"fmt"
 	"regexp"
 	"strings"
 	"unicode/utf8"
@@ -95,7 +94,7 @@ func (ns NamingStrategy) formatName(prefix, table, name string) string {
 		h.Write([]byte(formattedName))
 		bs := h.Sum(nil)
 
-		formattedName = fmt.Sprintf("%v%v%v", prefix, table, name)[0:56] + hex.EncodeToString(bs)[:8]
+		formattedName = formattedName[0:56] + hex.EncodeToString(bs)[:8]
 	}
 	return formattedName
 }


Test patch:
diff --git a/schema/naming_test.go b/schema/naming_test.go
index 1fdab9a..3f598c3 100644
--- a/schema/naming_test.go
+++ b/schema/naming_test.go
@@ -193,7 +193,7 @@ func TestFormatNameWithStringLongerThan64Characters(t *testing.T) {
 	ns := NamingStrategy{}
 
 	formattedName := ns.formatName("prefix", "table", "thisIsAVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString")
-	if formattedName != "prefixtablethisIsAVeryVeryVeryVeryVeryVeryVeryVeryVeryLo180f2c67" {
+	if formattedName != "prefix_table_thisIsAVeryVeryVeryVeryVeryVeryVeryVeryVery180f2c67" {
 		t.Errorf("invalid formatted name generated, got %v", formattedName)
 	}
 }
diff --git a/schema/relationship_test.go b/schema/relationship_test.go
index e2cf11a..40ffc32 100644
--- a/schema/relationship_test.go
+++ b/schema/relationship_test.go
@@ -576,3 +576,39 @@ func TestHasManySameForeignKey(t *testing.T) {
 		References: []Reference{{"ID", "User", "UserRefer", "Profile", "", true}},
 	})
 }
+
+type Author struct {
+	gorm.Model
+}
+
+type Book struct {
+	gorm.Model
+	Author   Author
+	AuthorID uint
+}
+
+func (Book) TableName() string {
+	return "my_schema.a_very_very_very_very_very_very_very_very_long_table_name"
+}
+
+func TestParseConstraintNameWithSchemaQualifiedLongTableName(t *testing.T) {
+	s, err := schema.Parse(
+		&Book{},
+		&sync.Map{},
+		schema.NamingStrategy{},
+	)
+	if err != nil {
+		t.Fatalf("Failed to parse schema")
+	}
+
+	expectedConstraintName := "fk_my_schema_a_very_very_very_very_very_very_very_very_l4db13eec"
+	constraint := s.Relationships.Relations["Author"].ParseConstraint()
+
+	if constraint.Name != expectedConstraintName {
+		t.Fatalf(
+			"expected constraint name %s, got %s",
+			expectedConstraintName,
+			constraint.Name,
+		)
+	}
+}