fix: The HasConstraint is fixed by adding a Unique Check#173
fix: The HasConstraint is fixed by adding a Unique Check#173jinzhu merged 3 commits intogo-gorm:masterfrom
Conversation
|
Hi @Orefa can you add some tests for the changes? |
|
There was a problem hiding this comment.
Pull request overview
This PR updates the SQL Server migrator so HasConstraint can detect UNIQUE constraints, and adds a regression test to cover the behavior.
Changes:
- Extend
Migrator.HasConstraintto also query SQL Server’s unique constraints (sys.key_constraintstypeUQ). - Refine
Migrator.ColumnTypesUNIQUE detection so only single-column UNIQUE constraints mark a column asUnique(). - Add a new test table + test case to validate
HasConstraintbehavior for a UNIQUE column.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| migrator.go | Adds UNIQUE constraint detection to HasConstraint and adjusts ColumnTypes UNIQUE flag inference. |
| migrator_test.go | Adds a regression test verifying HasConstraint returns true for a UNIQUE field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for columnTypeRows.Next() { | ||
| var constraintName string | ||
| columnTypeRows.Scan(&constraintName) | ||
| uniqueContraints[constraintName]++ | ||
| } |
There was a problem hiding this comment.
Row scanning errors are ignored when building the unique-constraint cardinality map (columnTypeRows.Scan(&constraintName)). If Scan fails, the map can be incomplete and later UniqueValue inference becomes wrong. Please handle the Scan error (and consider checking columnTypeRows.Err() after the loop) and return the error.
| for columnTypeRows.Next() { | ||
| var name, columnType string | ||
| _ = columnTypeRows.Scan(&name, &columnType) | ||
| var name, constraintName, columnType string | ||
| _ = columnTypeRows.Scan(&name, &constraintName, &columnType) | ||
| for idx, c := range columnTypes { | ||
| mc := c.(migrator.ColumnType) | ||
| if mc.NameValue.String == name { | ||
| switch columnType { | ||
| case "PRIMARY KEY": | ||
| mc.PrimaryKeyValue = sql.NullBool{Bool: true, Valid: true} | ||
| case "UNIQUE": | ||
| mc.UniqueValue = sql.NullBool{Bool: true, Valid: true} | ||
| if uniqueContraints[constraintName] == 1 { | ||
| mc.UniqueValue = sql.NullBool{Bool: true, Valid: true} | ||
| } |
There was a problem hiding this comment.
Row scanning errors are ignored when reading constraint metadata (_ = columnTypeRows.Scan(&name, &constraintName, &columnType)). If the scan fails, the loop will silently skip setting PrimaryKey/Unique flags. Please handle Scan errors (and optionally Rows.Err() after iteration) so ColumnTypes returns a deterministic error instead of silently degrading.
| query := "SELECT t.CONSTRAINT_NAME FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS t JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE c ON c.CONSTRAINT_NAME=t.CONSTRAINT_NAME WHERE t.CONSTRAINT_TYPE IN ('PRIMARY KEY', 'UNIQUE') AND c.TABLE_CATALOG = ? AND c.TABLE_NAME = ? AND CONSTRAINT_TYPE = ?" | ||
|
|
||
| queryParameters := []interface{}{m.CurrentDatabase(), tableName} | ||
|
|
||
| if schemaName != "" { | ||
| query += " AND c.TABLE_SCHEMA = ?" | ||
| queryParameters = append(queryParameters, schemaName) | ||
| } | ||
|
|
||
| queryParameters = append(queryParameters, "UNIQUE") |
There was a problem hiding this comment.
The UNIQUE-constraint prequery is redundant and slightly unclear: it filters t.CONSTRAINT_TYPE IN ('PRIMARY KEY','UNIQUE') and also adds AND CONSTRAINT_TYPE = ? (unqualified). Consider simplifying to a single, qualified predicate (e.g., t.CONSTRAINT_TYPE = 'UNIQUE') to avoid confusion and keep the query planner work minimal.
| uniqueContraints := map[string]int{} | ||
| for columnTypeRows.Next() { | ||
| var constraintName string | ||
| columnTypeRows.Scan(&constraintName) | ||
| uniqueContraints[constraintName]++ |
There was a problem hiding this comment.
Typo in variable name uniqueContraints; this reduces readability and makes future refactors/searching harder. Rename to uniqueConstraints.
add Unique Check