Skip to content

Commit ab567b6

Browse files
kvchkvch
authored
Add support for no_inherit on column level constraints and in create_constraint operation (#670)
This PR adds a new option `no_inherit` to inline and table level check constraints in `create_constraint`. It also makes sure that the check is duplicated correctly during migration. Also, I added the new attribute `noInherit` to the output of `analyze`/`read_schema`.
1 parent 8791a93 commit ab567b6

25 files changed

+241
-52
lines changed

‎docs/operations/create_constraint.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Required fields: `name`, `table`, `type`, `up`, `down`.
1717
"columns": ["column1", "column2"],
1818
"type": "unique"| "check" | "foreign_key",
1919
"check": "SQL expression for CHECK constraint",
20+
"no_inherit": "true|false",
2021
"references": {
2122
"name": "name of foreign key reference",
2223
"table": "name of referenced table",

‎docs/operations/create_table.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ Each `column` is defined as:
2828
"default": "default value",
2929
"check": {
3030
"name": "name of check constraint",
31-
"constraint": "constraint expression"
31+
"constraint": "constraint expression",
32+
"no_inherit": "true|false"
3233
},
3334
"generated": {
3435
"expression": "expression for stored column",

‎internal/jsonschema/testdata/create-constraint-1-invalid-check.txtar

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
This is an invalid 'create_constraint' migration.
2+
Check constraint must have the option 'check' configured.
3+
4+
-- create_constraint.json --
5+
{
6+
"name": "migration_name",
7+
"operations": [
8+
{
9+
"create_constraint": {
10+
"name": "my_invalid_check",
11+
"table": "my_table",
12+
"type": "check",
13+
"columns": [
14+
"my_column"
15+
],
16+
"up": {
17+
"my_column": "my_column"
18+
},
19+
"down": {
20+
"my_column": "my_column"
21+
}
22+
}
23+
}
24+
]
25+
}
26+
27+
-- valid --
28+
false

‎internal/jsonschema/testdata/create-constraint-2-invalid-no-inherit.txtar

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
This is an invalid 'create_constraint' migration.
2+
Only check constraints have no_inherit flag.
3+
4+
-- create_constraint.json --
5+
{
6+
"name": "migration_name",
7+
"operations": [
8+
{
9+
"create_constraint": {
10+
"name": "my_invalid_check",
11+
"table": "my_table",
12+
"type": "foreign_key",
13+
"no_inherit": true,
14+
"columns": [
15+
"my_column"
16+
],
17+
"up": {
18+
"my_column": "my_column"
19+
},
20+
"down": {
21+
"my_column": "my_column"
22+
}
23+
}
24+
}
25+
]
26+
}
27+
28+
-- valid --
29+
false

‎pkg/migrations/constraints.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,11 @@ func (w *ConstraintSQLWriter) WriteCheck(check string, noInherit bool) string {
5959
if w.Name != "" {
6060
constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name))
6161
}
62-
constraint += fmt.Sprintf("CHECK (%s)", check)
62+
if !strings.HasPrefix(check, "CHECK (") {
63+
constraint += fmt.Sprintf("CHECK (%s)", check)
64+
} else {
65+
constraint += check
66+
}
6367
if noInherit {
6468
constraint += " NO INHERIT"
6569
}

‎pkg/migrations/duplicate.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ type duplicatorStmtBuilder struct {
3939
const (
4040
dataTypeMismatchErrorCode pq.ErrorCode = "42804"
4141
undefinedFunctionErrorCode pq.ErrorCode = "42883"
42-
43-
cSetDefaultSQL = `ALTER TABLE %s ALTER COLUMN %s SET DEFAULT %s`
44-
cAlterTableAddCheckConstraintSQL = `ALTER TABLE %s ADD CONSTRAINT %s %s NOT VALID`
4542
)
4643

4744
// NewColumnDuplicator creates a new Duplicator for a column.
@@ -172,11 +169,10 @@ func (d *duplicatorStmtBuilder) duplicateCheckConstraints(withoutConstraint []st
172169
continue
173170
}
174171
if duplicatedConstraintColumns := d.duplicatedConstraintColumns(cc.Columns, colNames...); len(duplicatedConstraintColumns) > 0 {
175-
stmts = append(stmts, fmt.Sprintf(cAlterTableAddCheckConstraintSQL,
176-
pq.QuoteIdentifier(d.table.Name),
177-
pq.QuoteIdentifier(DuplicationName(cc.Name)),
178-
rewriteCheckExpression(cc.Definition, duplicatedConstraintColumns...),
179-
))
172+
sql := fmt.Sprintf("ALTER TABLE %s ADD ", pq.QuoteIdentifier(d.table.Name))
173+
writer := ConstraintSQLWriter{Name: DuplicationName(cc.Name), SkipValidation: true}
174+
sql += writer.WriteCheck(rewriteCheckExpression(cc.Definition, duplicatedConstraintColumns...), cc.NoInherit)
175+
stmts = append(stmts, sql)
180176
}
181177
}
182178
return stmts

‎pkg/migrations/duplicate_test.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88

99
"github.com/stretchr/testify/assert"
10+
pgq "github.com/xataio/pg_query_go/v6"
1011

1112
"github.com/xataio/pgroll/pkg/schema"
1213
)
@@ -27,7 +28,7 @@ var table = &schema.Table{
2728
"unique_name_nick": {Name: "unique_name_nick", Columns: []string{"name", "nick"}},
2829
},
2930
CheckConstraints: map[string]*schema.CheckConstraint{
30-
"email_at": {Name: "email_at", Columns: []string{"email"}, Definition: `"email" ~ '@'`},
31+
"email_at": {Name: "email_at", Columns: []string{"email"}, Definition: `"email" ~ '@'`, NoInherit: true},
3132
"adults": {Name: "adults", Columns: []string{"age"}, Definition: `"age" > 18`},
3233
"new_york_adults": {Name: "new_york_adults", Columns: []string{"city", "age"}, Definition: `"city" = 'New York' AND "age" > 21`},
3334
"different_nick": {Name: "different_nick", Columns: []string{"name", "nick"}, Definition: `"name" != "nick"`},
@@ -67,35 +68,37 @@ func TestDuplicateStmtBuilderCheckConstraints(t *testing.T) {
6768
},
6869
"single-column check constraint with single column duplicated": {
6970
columns: []string{"email"},
70-
expectedStmts: []string{`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_email_at" "_pgroll_new_email" ~ '@' NOT VALID`},
71+
expectedStmts: []string{`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_email_at" CHECK ("_pgroll_new_email" ~ '@') NO INHERIT NOT VALID`},
7172
},
7273
"multiple multi and single column check constraint with single column duplicated": {
7374
columns: []string{"age"},
7475
expectedStmts: []string{
75-
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_adults" "_pgroll_new_age" > 18 NOT VALID`,
76-
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_new_york_adults" "city" = 'New York' AND "_pgroll_new_age" > 21 NOT VALID`,
76+
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_adults" CHECK ("_pgroll_new_age" > 18) NOT VALID`,
77+
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_new_york_adults" CHECK ("city" = 'New York' AND "_pgroll_new_age" > 21) NOT VALID`,
7778
},
7879
},
7980
"multiple multi and single column check constraint with multiple column duplicated": {
8081
columns: []string{"age", "description"},
8182
expectedStmts: []string{
82-
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_adults" "_pgroll_new_age" > 18 NOT VALID`,
83-
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_new_york_adults" "city" = 'New York' AND "_pgroll_new_age" > 21 NOT VALID`,
83+
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_adults" CHECK ("_pgroll_new_age" > 18) NOT VALID`,
84+
`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_new_york_adults" CHECK ("city" = 'New York' AND "_pgroll_new_age" > 21) NOT VALID`,
8485
},
8586
},
8687
"multi-column check constraint with multiple columns with single column duplicated": {
8788
columns: []string{"name"},
88-
expectedStmts: []string{`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_different_nick" "_pgroll_new_name" != "nick" NOT VALID`},
89+
expectedStmts: []string{`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_different_nick" CHECK ("_pgroll_new_name" != "nick") NOT VALID`},
8990
},
9091
"multi-column check constraint with multiple columns duplicated": {
9192
columns: []string{"name", "nick"},
92-
expectedStmts: []string{`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_different_nick" "_pgroll_new_name" != "_pgroll_new_nick" NOT VALID`},
93+
expectedStmts: []string{`ALTER TABLE "test_table" ADD CONSTRAINT "_pgroll_dup_different_nick" CHECK ("_pgroll_new_name" != "_pgroll_new_nick") NOT VALID`},
9394
},
9495
} {
9596
t.Run(name, func(t *testing.T) {
9697
stmts := d.duplicateCheckConstraints(nil, testCases.columns...)
9798
assert.Equal(t, len(testCases.expectedStmts), len(stmts))
9899
for _, stmt := range stmts {
100+
_, err := pgq.Parse(stmt)
101+
assert.NoError(t, err)
99102
assert.True(t, slices.Contains(testCases.expectedStmts, stmt))
100103
}
101104
})

‎pkg/migrations/op_add_column.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,9 +371,8 @@ func (w ColumnSQLWriter) Write(col Column) (string, error) {
371371
col.References.MatchType)
372372
}
373373
if col.Check != nil {
374-
sql += fmt.Sprintf(" CONSTRAINT %s CHECK (%s)",
375-
pq.QuoteIdentifier(col.Check.Name),
376-
col.Check.Constraint)
374+
writer := &ConstraintSQLWriter{Name: col.Check.Name}
375+
sql += " " + writer.WriteCheck(col.Check.Constraint, col.Check.NoInherit)
377376
}
378377
return sql, nil
379378
}

‎pkg/migrations/op_common_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,21 @@ func TriggerMustNotExist(t *testing.T, db *sql.DB, schema, table, trigger string
227227

228228
func CheckConstraintMustNotExist(t *testing.T, db *sql.DB, schema, table, constraint string) {
229229
t.Helper()
230-
if checkConstraintExists(t, db, schema, table, constraint) {
230+
if checkConstraintExists(t, db, schema, table, constraint, false) {
231231
t.Fatalf("Expected constraint %q to not exist", constraint)
232232
}
233233
}
234234

235235
func CheckConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) {
236236
t.Helper()
237-
if !checkConstraintExists(t, db, schema, table, constraint) {
237+
if !checkConstraintExists(t, db, schema, table, constraint, false) {
238+
t.Fatalf("Expected constraint %q to exist", constraint)
239+
}
240+
}
241+
242+
func NotInheritableCheckConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) {
243+
t.Helper()
244+
if !checkConstraintExists(t, db, schema, table, constraint, true) {
238245
t.Fatalf("Expected constraint %q to exist", constraint)
239246
}
240247
}
@@ -371,7 +378,7 @@ func CheckIndexDefinition(t *testing.T, db *sql.DB, schema, table, index, expect
371378
}
372379
}
373380

374-
func checkConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint string) bool {
381+
func checkConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint string, noInherit bool) bool {
375382
t.Helper()
376383

377384
var exists bool
@@ -382,8 +389,9 @@ func checkConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint s
382389
WHERE conrelid = $1::regclass
383390
AND conname = $2
384391
AND contype = 'c'
392+
AND connoinherit = $3
385393
)`,
386-
fmt.Sprintf("%s.%s", schema, table), constraint).Scan(&exists)
394+
fmt.Sprintf("%s.%s", schema, table), constraint, noInherit).Scan(&exists)
387395
if err != nil {
388396
t.Fatal(err)
389397
}

‎pkg/migrations/op_create_constraint.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,12 +265,14 @@ func (o *OpCreateConstraint) Validate(ctx context.Context, s *schema.Schema) err
265265
}
266266

267267
func (o *OpCreateConstraint) addCheckConstraint(ctx context.Context, conn db.DB, tableName string) error {
268-
_, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s) NOT VALID",
269-
pq.QuoteIdentifier(tableName),
270-
pq.QuoteIdentifier(o.Name),
271-
rewriteCheckExpression(*o.Check, o.Columns...),
272-
))
268+
sql := fmt.Sprintf("ALTER TABLE %s ADD ", pq.QuoteIdentifier(tableName))
273269

270+
writer := &ConstraintSQLWriter{
271+
Name: o.Name,
272+
SkipValidation: true,
273+
}
274+
sql += writer.WriteCheck(rewriteCheckExpression(*o.Check, o.Columns...), o.NoInherit)
275+
_, err := conn.ExecContext(ctx, sql)
274276
return err
275277
}
276278

0 commit comments

Comments
 (0)