From 5908c19f9372c59a2dfa77d4f6a086b3b8c668ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 9 Jan 2025 16:13:44 +0100 Subject: [PATCH 01/66] mi folyik itt gyongyoson? --- schema.json | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/schema.json b/schema.json index 4a35ee072..9e8c73ad9 100644 --- a/schema.json +++ b/schema.json @@ -98,6 +98,58 @@ "type": "string", "enum": ["NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET DEFAULT"] }, + "Constraint": { + "additionalProperties": false, + "description": "Constraint definition", + "properties": { + "name": { + "description": "Name of the constraint", + "type": "string" + }, + "columns": { + "description": "Columns to add constraint to", + "type": "array", + "items": { + "type": "string" + } + }, + "type": { + "description": "Type of the constraint", + "type": "string", + "enum": ["unique", "check", "foreign_key"] + }, + "check": { + "description": "Check constraint expression", + "type": "string" + }, + "references": { + "description": "Reference to the foreign key", + "type": "object", + "additionalProperties": false, + "required": ["table", "columns"], + "properties": { + "table": { + "description": "Name of the table", + "type": "string" + }, + "columns": { + "description": "Columns to reference", + "type": "array", + "items": { + "type": "string" + } + }, + "on_delete": { + "description": "On delete behavior of the foreign key constraint", + "$ref": "#/$defs/ForeignKeyReferenceOnDelete", + "default": "NO ACTION" + } + } + } + }, + "required": ["name", "type"], + "type": "object" + }, "OpAddColumn": { "additionalProperties": false, "description": "Add column operation", @@ -279,6 +331,13 @@ "comment": { "description": "Postgres comment for the table", "type": "string" + }, + "constraints": { + "items": { + "$ref": "#/$defs/Constraint", + "description": "Constraints to add to the table" + }, + "type": "array" } }, "required": ["columns", "name"], From c9ef10d49b74de6fdf1550f6e82839ecfb53e908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 9 Jan 2025 17:10:37 +0100 Subject: [PATCH 02/66] update generated file --- pkg/migrations/types.go | 83 +++++++++++++++++++++++++++++++++++++++++ schema.json | 58 +++++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 888d19091..772bb0663 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -44,6 +44,86 @@ type Column struct { Unique bool `json:"unique,omitempty"` } +// Constraint definition +type Constraint struct { + // Check constraint expression + Check *string `json:"check,omitempty"` + + // Columns to add constraint to + Columns []string `json:"columns,omitempty"` + + // Defferable constraint + Defferable *bool `json:"defferable,omitempty"` + + // Exclude constraint + Exclude *ConstraintExclude `json:"exclude,omitempty"` + + // Initially deferred constraint + InitiallyDeferred *bool `json:"initially_deferred,omitempty"` + + // Name of the constraint + Name string `json:"name"` + + // No inherit constraint + NoInherit *bool `json:"no_inherit,omitempty"` + + // Nulls not distinct constraint + NullsNotDistinct *bool `json:"nulls_not_distinct,omitempty"` + + // Reference to the foreign key + References *ConstraintReferences `json:"references,omitempty"` + + // Type of the constraint + Type ConstraintType `json:"type"` +} + +// Exclude constraint +type ConstraintExclude struct { + // Elements corresponds to the JSON schema field "elements". + Elements []string `json:"elements,omitempty"` + + // IncludeColumns corresponds to the JSON schema field "include_columns". + IncludeColumns []string `json:"include_columns,omitempty"` + + // IndexMethod corresponds to the JSON schema field "index_method". + IndexMethod *string `json:"index_method,omitempty"` + + // Predicate corresponds to the JSON schema field "predicate". + Predicate *string `json:"predicate,omitempty"` + + // StorageParameters corresponds to the JSON schema field "storage_parameters". + StorageParameters *string `json:"storage_parameters,omitempty"` + + // Tablespace corresponds to the JSON schema field "tablespace". + Tablespace *string `json:"tablespace,omitempty"` +} + +// Reference to the foreign key +type ConstraintReferences struct { + // Columns to reference + Columns []string `json:"columns"` + + // Match type of the foreign key constraint + MatchType *string `json:"match_type,omitempty"` + + // On delete behavior of the foreign key constraint + OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` + + // On update behavior of the foreign key constraint + OnUpdate ForeignKeyReferenceOnDelete `json:"on_update,omitempty"` + + // Name of the table + Table string `json:"table"` +} + +type ConstraintType string + +const ConstraintTypeCheck ConstraintType = "check" +const ConstraintTypeExclude ConstraintType = "exclude" +const ConstraintTypeForeignKey ConstraintType = "foreign_key" +const ConstraintTypePrimaryKey ConstraintType = "primary_key" +const ConstraintTypeUnique ConstraintType = "unique" + // Foreign key reference definition type ForeignKeyReference struct { // Name of the referenced column @@ -212,6 +292,9 @@ type OpCreateTable struct { // Postgres comment for the table Comment *string `json:"comment,omitempty"` + // Constraints corresponds to the JSON schema field "constraints". + Constraints []Constraint `json:"constraints,omitempty"` + // Name of the table Name string `json:"name"` } diff --git a/schema.json b/schema.json index 9e8c73ad9..a462952ec 100644 --- a/schema.json +++ b/schema.json @@ -116,7 +116,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["unique", "check", "foreign_key"] + "enum": ["unique", "check", "primary_key", "foreign_key", "exclude"] }, "check": { "description": "Check constraint expression", @@ -143,6 +143,62 @@ "description": "On delete behavior of the foreign key constraint", "$ref": "#/$defs/ForeignKeyReferenceOnDelete", "default": "NO ACTION" + }, + "on_update": { + "description": "On update behavior of the foreign key constraint", + "$ref": "#/$defs/ForeignKeyReferenceOnDelete", + "default": "NO ACTION" + }, + "match_type": { + "description": "Match type of the foreign key constraint", + "type": "string" + } + } + }, + "defferable": { + "description": "Defferable constraint", + "type": "boolean" + }, + "initially_deferred": { + "description": "Initially deferred constraint", + "type": "boolean" + }, + "no_inherit": { + "description": "No inherit constraint", + "type": "boolean" + }, + "nulls_not_distinct": { + "description": "Nulls not distinct constraint", + "type": "boolean" + }, + "exclude": { + "description": "Exclude constraint", + "additionalProperties": false, + "type": "object", + "properties": { + "elements": { + "type": "array", + "items": { + "type": "string" + } + }, + "index_method": { + "type": "string" + }, + "include_columns": { + "type": "array", + "items": { + "type": "string" + } + }, + "storage_parameters": { + "type": "string" + }, + "predicate": { + "type": "string" + }, + "tablespace": { + "type": "string" } } } From 03b3952a818df8803685df05d6cbc06b752ed5f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 9 Jan 2025 20:12:06 +0100 Subject: [PATCH 03/66] first batch --- pkg/migrations/op_create_table.go | 97 ++++++++++++++++++++++++++++++- pkg/migrations/types.go | 26 ++++----- pkg/sql2pgroll/create_table.go | 94 +++++++++++++++++++++++++++++- schema.json | 36 ++++++------ 4 files changed, 217 insertions(+), 36 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 833c6d211..cec56f171 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -22,10 +22,16 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri return nil, fmt.Errorf("failed to create columns SQL: %w", err) } + constraintsSQL, err := constraintsToSQL(o.Constraints) + if err != nil { + return nil, fmt.Errorf("failed to create constraints SQL: %w", err) + } + // Create the table - _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s)", + _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s %s)", pq.QuoteIdentifier(o.Name), - columnsSQL)) + columnsSQL, + constraintsSQL)) if err != nil { return nil, err } @@ -150,3 +156,90 @@ func columnsToSQL(cols []Column, tr SQLTransformer) (string, error) { } return sql, nil } + +func constraintsToSQL(constraints []Constraint) (string, error) { + constraintsSQL := make([]string, len(constraints)) + for i, c := range constraints { + switch c.Type { + case ConstraintTypeCheck: + constraintsSQL[i] = fmt.Sprintf("CONSTRAINT %s CHECK (%s)", c.Name, *c.Check) + case ConstraintTypeExclude: + writer := &ConstraintSQLWriter{ + Name: c.Name, + Columns: c.Columns, + IncludeColumns: c.IncludeColumns, + StorageParameters: *c.StorageParameters, + Tablespace: *c.Tablespace, + } + constraintsSQL[i] = writer.WriteExclude(c.Exclude.IndexMethod, c.Exclude.Elements) + case ConstraintTypeForeignKey: + constraintsSQL[i] = fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s)", c.Name, c.Columns, c.References.Table, c.References.Columns) + // TODO foreign key options + case ConstraintTypePrimaryKey: + writer := &ConstraintSQLWriter{ + Name: c.Name, + Columns: c.Columns, + IncludeColumns: c.IncludeColumns, + StorageParameters: *c.StorageParameters, + Tablespace: *c.Tablespace, + } + constraintsSQL[i] = writer.WritePrimaryKey() + case ConstraintTypeUnique: + writer := &ConstraintSQLWriter{ + Name: c.Name, + Columns: c.Columns, + IncludeColumns: c.IncludeColumns, + StorageParameters: *c.StorageParameters, + Tablespace: *c.Tablespace, + } + constraintsSQL[i] = writer.WriteUnique(*c.NullsNotDistinct) + } + } + return strings.Join(constraintsSQL, ", "), nil +} + +type ConstraintSQLWriter struct { + Name string + Columns []string + + // unique, exclude, primary key constraints support the following options + IncludeColumns []string + StorageParameters string + Tablespace string +} + +func (w *ConstraintSQLWriter) WriteExclude(indexMethod, elements string) string { + constraint := fmt.Sprintf("EXCLUDE USING %s (%s)", indexMethod, elements) + constraint += w.addIndexParameters() + return constraint +} + +func (w *ConstraintSQLWriter) WritePrimaryKey() string { + constraint := fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", w.Name, strings.Join(quoteColumnNames(w.Columns), ", ")) + constraint += w.addIndexParameters() + return "" +} + +func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct bool) string { + nullsDistinct := "" + if nullsNotDistinct { + nullsDistinct = "NULLS NOT DISTINCT" + } + constraint := fmt.Sprintf("CONSTRAINT %s UNIQUE %s (%s)", w.Name, nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) + constraint += w.addIndexParameters() + return constraint +} + +func (w *ConstraintSQLWriter) addIndexParameters() string { + constraint := "" + if len(w.IncludeColumns) != 0 { + constraint += fmt.Sprintf(" INCLUDE (%s)", strings.Join(quoteColumnNames(w.IncludeColumns), ", ")) + } + if w.StorageParameters != "" { + constraint += fmt.Sprintf(" WITH (%s)", w.StorageParameters) + } + if w.Tablespace != "" { + constraint += fmt.Sprintf(" USING INDEX TABLESPACE %s", w.Tablespace) + } + return constraint +} diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 772bb0663..53980eb9f 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -52,12 +52,15 @@ type Constraint struct { // Columns to add constraint to Columns []string `json:"columns,omitempty"` - // Defferable constraint - Defferable *bool `json:"defferable,omitempty"` + // Deferable constraint + Deferable *bool `json:"deferable,omitempty"` // Exclude constraint Exclude *ConstraintExclude `json:"exclude,omitempty"` + // IncludeColumns corresponds to the JSON schema field "include_columns". + IncludeColumns []string `json:"include_columns,omitempty"` + // Initially deferred constraint InitiallyDeferred *bool `json:"initially_deferred,omitempty"` @@ -73,6 +76,12 @@ type Constraint struct { // Reference to the foreign key References *ConstraintReferences `json:"references,omitempty"` + // StorageParameters corresponds to the JSON schema field "storage_parameters". + StorageParameters *string `json:"storage_parameters,omitempty"` + + // Tablespace corresponds to the JSON schema field "tablespace". + Tablespace *string `json:"tablespace,omitempty"` + // Type of the constraint Type ConstraintType `json:"type"` } @@ -80,22 +89,13 @@ type Constraint struct { // Exclude constraint type ConstraintExclude struct { // Elements corresponds to the JSON schema field "elements". - Elements []string `json:"elements,omitempty"` - - // IncludeColumns corresponds to the JSON schema field "include_columns". - IncludeColumns []string `json:"include_columns,omitempty"` + Elements string `json:"elements"` // IndexMethod corresponds to the JSON schema field "index_method". - IndexMethod *string `json:"index_method,omitempty"` + IndexMethod string `json:"index_method"` // Predicate corresponds to the JSON schema field "predicate". Predicate *string `json:"predicate,omitempty"` - - // StorageParameters corresponds to the JSON schema field "storage_parameters". - StorageParameters *string `json:"storage_parameters,omitempty"` - - // Tablespace corresponds to the JSON schema field "tablespace". - Tablespace *string `json:"tablespace,omitempty"` } // Reference to the foreign key diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 74a5792fc..098dabe10 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -4,6 +4,7 @@ package sql2pgroll import ( "fmt" + "strings" pgq "github.com/xataio/pg_query_go/v6" @@ -38,10 +39,20 @@ func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { } } + var constraints []migrations.Constraint + for _, c := range stmt.Constraints { + constraint, err := convertConstraint(c.GetConstraint()) + if err != nil { + return nil, fmt.Errorf("error converting table constraint: %w", err) + } + constraints = append(constraints, *constraint) + } + return migrations.Operations{ &migrations.OpCreateTable{ - Name: getQualifiedRelationName(stmt.GetRelation()), - Columns: columns, + Name: getQualifiedRelationName(stmt.GetRelation()), + Columns: columns, + Constraints: constraints, }, }, nil } @@ -189,6 +200,85 @@ func convertColumnDef(tableName string, col *pgq.ColumnDef) (*migrations.Column, }, nil } +func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { + var constraintType migrations.ConstraintType + var nullsNotDistinct *bool + var exclude *migrations.ConstraintExclude + + switch c.Contype { + case pgq.ConstrType_CONSTR_UNIQUE: + constraintType = migrations.ConstraintTypeUnique + nullsNotDistinct = ptr(c.NullsNotDistinct) + case pgq.ConstrType_CONSTR_EXCLUSION: + exclusionElemens := make([]string, len(c.Exclusions)) + for i, ex := range c.Exclusions { + if len(ex.GetList().Items) != 2 { + return nil, fmt.Errorf("unexpected number of elements in exclusion constraint: %d", len(ex.GetList().Items)) + } + colName := ex.GetList().Items[0].GetIndexElem().Name + opName := ex.GetList().Items[1].GetString_().Sval + exclusionElemens[i] = fmt.Sprintf("%s WITH %s", colName, opName) + } + exclude = &migrations.ConstraintExclude{ + Elements: strings.Join(exclusionElemens, ", "), + IndexMethod: c.AccessMethod, + } + case pgq.ConstrType_CONSTR_PRIMARY: + constraintType = migrations.ConstraintTypePrimaryKey + case pgq.ConstrType_CONSTR_FOREIGN: + constraintType = migrations.ConstraintTypeForeignKey + case pgq.ConstrType_CONSTR_CHECK: + constraintType = migrations.ConstraintTypeCheck + default: + return nil, fmt.Errorf("unsupported constraint type: %s", c.Contype) + } + + including := make([]string, len(c.Including)) + for i, include := range c.Including { + including[i] = include.GetString_().Sval + } + + options := make([]string, len(c.Options)) + for i, option := range c.Options { + var val string + switch v := option.GetDefElem().Arg.GetNode().(type) { + case *pgq.Node_Float: + val = v.Float.GetFval() + case *pgq.Node_Integer: + val = fmt.Sprintf("%d", v.Integer.GetIval()) + case *pgq.Node_String_: + val = v.String_.GetSval() + case *pgq.Node_Boolean: + val = v.Boolean.String() + default: + return nil, fmt.Errorf("unsupported storage parameter type: %T", v) + } + options[i] = fmt.Sprintf("%s = '%s'", option.GetDefElem().Defname, val) + } + var storageParameters *string + if len(options) != 0 { + storageParameters = ptr(strings.Join(options, ", ")) + } + + var tablespace *string + if c.Indexspace != "" { + tablespace = ptr(c.Indexspace) + } + + return &migrations.Constraint{ + Name: c.Conname, + Type: constraintType, + NullsNotDistinct: nullsNotDistinct, + Deferable: ptr(c.Deferrable), + InitiallyDeferred: ptr(c.Initdeferred), + NoInherit: ptr(c.IsNoInherit), + StorageParameters: storageParameters, + Tablespace: tablespace, + IncludeColumns: including, + Exclude: exclude, + }, nil +} + // canConvertColumnDef returns true iff `col` can be converted to a pgroll // `Column` definition. func canConvertColumnDef(col *pgq.ColumnDef) bool { diff --git a/schema.json b/schema.json index a462952ec..510b3453c 100644 --- a/schema.json +++ b/schema.json @@ -155,8 +155,8 @@ } } }, - "defferable": { - "description": "Defferable constraint", + "deferable": { + "description": "Deferable constraint", "type": "boolean" }, "initially_deferred": { @@ -171,36 +171,34 @@ "description": "Nulls not distinct constraint", "type": "boolean" }, + "tablespace": { + "type": "string" + }, + "storage_parameters": { + "type": "string" + }, + "include_columns": { + "type": "array", + "items": { + "type": "string" + } + }, "exclude": { "description": "Exclude constraint", "additionalProperties": false, "type": "object", "properties": { "elements": { - "type": "array", - "items": { - "type": "string" - } - }, - "index_method": { "type": "string" }, - "include_columns": { - "type": "array", - "items": { - "type": "string" - } - }, - "storage_parameters": { + "index_method": { "type": "string" }, "predicate": { "type": "string" - }, - "tablespace": { - "type": "string" } - } + }, + "required": ["index_method", "elements"] } }, "required": ["name", "type"], From f84b9a7ab63ac1c5f71d7a0c573f246194b75714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 13:35:22 +0100 Subject: [PATCH 04/66] more updates --- pkg/migrations/op_create_table.go | 123 +++++++++++++++++++++++------- 1 file changed, 94 insertions(+), 29 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index cec56f171..5abf7e49f 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -108,6 +108,21 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { } } + primaryFound := false + for _, c := range o.Constraints { + if err := ValidateIdentifierLength(c.Name); err != nil { + return fmt.Errorf("invalid constraint: %w", err) + } + + switch c.Type { + case ConstraintTypePrimaryKey: + if primaryFound { + return fmt.Errorf("multiple primary key constraints defined for table %s", o.Name) + } + primaryFound = true + } + } + // Update the schema to ensure that the new table is visible to validation of // subsequent operations. o.updateSchema(s) @@ -119,14 +134,58 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { // the new table. func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { columns := make(map[string]*schema.Column, len(o.Columns)) + var primaryKeys []string for _, col := range o.Columns { columns[col.Name] = &schema.Column{ Name: col.Name, } + if col.Pk { + primaryKeys = append(primaryKeys, col.Name) + } + } + var uniqueConstraints map[string]*schema.UniqueConstraint + var checkConstraints map[string]*schema.CheckConstraint + var foreignKeys map[string]schema.ForeignKey + for _, c := range o.Constraints { + switch c.Type { + case ConstraintTypeUnique: + if uniqueConstraints == nil { + uniqueConstraints = make(map[string]*schema.UniqueConstraint) + } + uniqueConstraints[c.Name] = &schema.UniqueConstraint{ + Name: c.Name, + Columns: c.Columns, + } + case ConstraintTypeCheck: + if checkConstraints == nil { + checkConstraints = make(map[string]*schema.CheckConstraint) + } + checkConstraints[c.Name] = &schema.CheckConstraint{ + Name: c.Name, + Columns: c.Columns, + Definition: *c.Check, + } + case ConstraintTypePrimaryKey: + primaryKeys = c.Columns + case ConstraintTypeForeignKey: + if foreignKeys == nil { + foreignKeys = make(map[string]schema.ForeignKey) + } + foreignKeys[c.Name] = schema.ForeignKey{ + Name: c.Name, + Columns: c.Columns, + ReferencedTable: c.References.Table, + ReferencedColumns: c.References.Columns, + OnDelete: string(c.References.OnDelete), + } + } } s.AddTable(o.Name, &schema.Table{ - Name: o.Name, - Columns: columns, + Name: o.Name, + Columns: columns, + PrimaryKey: primaryKeys, + CheckConstraints: checkConstraints, + UniqueConstraints: uniqueConstraints, }) return s @@ -160,38 +219,24 @@ func columnsToSQL(cols []Column, tr SQLTransformer) (string, error) { func constraintsToSQL(constraints []Constraint) (string, error) { constraintsSQL := make([]string, len(constraints)) for i, c := range constraints { + writer := &ConstraintSQLWriter{ + Name: c.Name, + Columns: c.Columns, + IncludeColumns: c.IncludeColumns, + StorageParameters: *c.StorageParameters, + Tablespace: *c.Tablespace, + } + switch c.Type { case ConstraintTypeCheck: - constraintsSQL[i] = fmt.Sprintf("CONSTRAINT %s CHECK (%s)", c.Name, *c.Check) + constraintsSQL[i] = writer.WriteCheck(*c.Check) case ConstraintTypeExclude: - writer := &ConstraintSQLWriter{ - Name: c.Name, - Columns: c.Columns, - IncludeColumns: c.IncludeColumns, - StorageParameters: *c.StorageParameters, - Tablespace: *c.Tablespace, - } constraintsSQL[i] = writer.WriteExclude(c.Exclude.IndexMethod, c.Exclude.Elements) case ConstraintTypeForeignKey: - constraintsSQL[i] = fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s)", c.Name, c.Columns, c.References.Table, c.References.Columns) - // TODO foreign key options + constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete) case ConstraintTypePrimaryKey: - writer := &ConstraintSQLWriter{ - Name: c.Name, - Columns: c.Columns, - IncludeColumns: c.IncludeColumns, - StorageParameters: *c.StorageParameters, - Tablespace: *c.Tablespace, - } constraintsSQL[i] = writer.WritePrimaryKey() case ConstraintTypeUnique: - writer := &ConstraintSQLWriter{ - Name: c.Name, - Columns: c.Columns, - IncludeColumns: c.IncludeColumns, - StorageParameters: *c.StorageParameters, - Tablespace: *c.Tablespace, - } constraintsSQL[i] = writer.WriteUnique(*c.NullsNotDistinct) } } @@ -208,14 +253,34 @@ type ConstraintSQLWriter struct { Tablespace string } +func (w *ConstraintSQLWriter) WriteCheck(check string) string { + constraint := fmt.Sprintf("CONSTRAINT %s CHECK (%s)", pq.QuoteIdentifier(w.Name), check) + return constraint +} + func (w *ConstraintSQLWriter) WriteExclude(indexMethod, elements string) string { - constraint := fmt.Sprintf("EXCLUDE USING %s (%s)", indexMethod, elements) + constraint := fmt.Sprintf("CONSTRAINT %s EXCLUDE USING %s (%s)", pq.QuoteIdentifier(w.Name), indexMethod, elements) constraint += w.addIndexParameters() return constraint } +func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDeleteAction ForeignKeyReferenceOnDelete) string { + onDelete := "NO ACTION" + if onDeleteAction != "" { + onDelete = strings.ToUpper(string(onDeleteAction)) + } + constraint := fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s", + pq.QuoteIdentifier(w.Name), + strings.Join(quoteColumnNames(w.Columns), ", "), + pq.QuoteIdentifier(referencedTable), + strings.Join(quoteColumnNames(referencedColumns), ", "), + onDelete, + ) + return constraint +} + func (w *ConstraintSQLWriter) WritePrimaryKey() string { - constraint := fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", w.Name, strings.Join(quoteColumnNames(w.Columns), ", ")) + constraint := fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", pq.QuoteIdentifier(w.Name), strings.Join(quoteColumnNames(w.Columns), ", ")) constraint += w.addIndexParameters() return "" } @@ -225,7 +290,7 @@ func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct bool) string { if nullsNotDistinct { nullsDistinct = "NULLS NOT DISTINCT" } - constraint := fmt.Sprintf("CONSTRAINT %s UNIQUE %s (%s)", w.Name, nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) + constraint := fmt.Sprintf("CONSTRAINT %s UNIQUE %s (%s)", pq.QuoteIdentifier(w.Name), nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) constraint += w.addIndexParameters() return constraint } From 296731ed1038bc279689cabc583c059fc65726d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 13:49:24 +0100 Subject: [PATCH 05/66] validation --- pkg/migrations/op_create_table.go | 40 +++++++++++++++++++++++++++---- pkg/sql2pgroll/create_table.go | 6 ++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 5abf7e49f..c048d5532 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -110,6 +110,9 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { primaryFound := false for _, c := range o.Constraints { + if c.Name == "" { + return FieldRequiredError{Name: "name"} + } if err := ValidateIdentifierLength(c.Name); err != nil { return fmt.Errorf("invalid constraint: %w", err) } @@ -120,6 +123,28 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { return fmt.Errorf("multiple primary key constraints defined for table %s", o.Name) } primaryFound = true + if len(c.Columns) == 0 { + return FieldRequiredError{Name: "columns"} + } + case ConstraintTypeUnique: + if len(c.Columns) == 0 { + return FieldRequiredError{Name: "columns"} + } + case ConstraintTypeCheck: + if c.Check == nil { + return FieldRequiredError{Name: "check"} + } + case ConstraintTypeForeignKey: + if c.References == nil { + return FieldRequiredError{Name: "references"} + } + case ConstraintTypeExclude: + if c.Exclude == nil { + return FieldRequiredError{Name: "exclude"} + } + if c.Exclude.Elements == "" { + return FieldRequiredError{Name: "exclude.elements"} + } } } @@ -233,7 +258,7 @@ func constraintsToSQL(constraints []Constraint) (string, error) { case ConstraintTypeExclude: constraintsSQL[i] = writer.WriteExclude(c.Exclude.IndexMethod, c.Exclude.Elements) case ConstraintTypeForeignKey: - constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete) + constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete, c.References.OnUpdate) case ConstraintTypePrimaryKey: constraintsSQL[i] = writer.WritePrimaryKey() case ConstraintTypeUnique: @@ -264,17 +289,22 @@ func (w *ConstraintSQLWriter) WriteExclude(indexMethod, elements string) string return constraint } -func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDeleteAction ForeignKeyReferenceOnDelete) string { - onDelete := "NO ACTION" +func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDeleteAction, onUpdateAction ForeignKeyReferenceOnDelete) string { + onDelete := string(ForeignKeyReferenceOnDeleteNOACTION) if onDeleteAction != "" { onDelete = strings.ToUpper(string(onDeleteAction)) } - constraint := fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s", + onUpdate := string(ForeignKeyReferenceOnDeleteNOACTION) + if onUpdateAction != "" { + onUpdate = strings.ToUpper(string(onUpdateAction)) + } + constraint := fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s ON UPDATE %s", pq.QuoteIdentifier(w.Name), strings.Join(quoteColumnNames(w.Columns), ", "), pq.QuoteIdentifier(referencedTable), strings.Join(quoteColumnNames(referencedColumns), ", "), onDelete, + onUpdate, ) return constraint } @@ -282,7 +312,7 @@ func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referenced func (w *ConstraintSQLWriter) WritePrimaryKey() string { constraint := fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", pq.QuoteIdentifier(w.Name), strings.Join(quoteColumnNames(w.Columns), ", ")) constraint += w.addIndexParameters() - return "" + return constraint } func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct bool) string { diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 098dabe10..c30f97aaa 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -39,13 +39,13 @@ func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { } } - var constraints []migrations.Constraint - for _, c := range stmt.Constraints { + constraints := make([]migrations.Constraint, len(stmt.Constraints)) + for i, c := range stmt.Constraints { constraint, err := convertConstraint(c.GetConstraint()) if err != nil { return nil, fmt.Errorf("error converting table constraint: %w", err) } - constraints = append(constraints, *constraint) + constraints[i] = *constraint } return migrations.Operations{ From 766ddbc8296c962a1bf6c1b93f989101520eae05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 15:07:50 +0100 Subject: [PATCH 06/66] add table constraint --- examples/.ledger | 1 + ...50_create_table_with_table_constraint.json | 39 +++++ pkg/migrations/op_create_table.go | 159 ++++++------------ pkg/migrations/op_create_table_test.go | 64 +++++++ pkg/migrations/types.go | 61 ++----- pkg/sql2pgroll/create_table.go | 37 ++-- pkg/sql2pgroll/create_table_test.go | 10 +- pkg/sql2pgroll/expect/create_table.go | 50 ++++++ schema.json | 76 ++------- 9 files changed, 242 insertions(+), 255 deletions(-) create mode 100644 examples/50_create_table_with_table_constraint.json diff --git a/examples/.ledger b/examples/.ledger index c59c07f57..9f415628e 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -47,3 +47,4 @@ 47_add_table_foreign_key_constraint.json 48_drop_tickets_check.json 49_unset_not_null_on_indexed_column.json +50_create_table_with_table_constraint.json diff --git a/examples/50_create_table_with_table_constraint.json b/examples/50_create_table_with_table_constraint.json new file mode 100644 index 000000000..a90ea6772 --- /dev/null +++ b/examples/50_create_table_with_table_constraint.json @@ -0,0 +1,39 @@ +{ + "name": "50_create_table_with_table_constraint", + "operations": [ + { + "create_table": { + "name": "phonebook", + "columns": [ + { + "name": "id", + "type": "serial", + "pk": true + }, + { + "name": "name", + "type": "varchar(255)" + }, + { + "name": "city", + "type": "varchar(255)" + }, + { + "name": "phone", + "type": "varchar(255)" + } + ], + "constraints": [ + { + "name": "unique_numbers", + "type": "unique", + "columns": ["phone"], + "index_parameters": { + "include_columns": ["name"] + } + } + ] + } + } + ] +} diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index c048d5532..60da7ed20 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -108,7 +108,6 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { } } - primaryFound := false for _, c := range o.Constraints { if c.Name == "" { return FieldRequiredError{Name: "name"} @@ -117,34 +116,11 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { return fmt.Errorf("invalid constraint: %w", err) } - switch c.Type { - case ConstraintTypePrimaryKey: - if primaryFound { - return fmt.Errorf("multiple primary key constraints defined for table %s", o.Name) - } - primaryFound = true - if len(c.Columns) == 0 { - return FieldRequiredError{Name: "columns"} - } + switch c.Type { //nolint:gocritic // more cases are coming soon case ConstraintTypeUnique: if len(c.Columns) == 0 { return FieldRequiredError{Name: "columns"} } - case ConstraintTypeCheck: - if c.Check == nil { - return FieldRequiredError{Name: "check"} - } - case ConstraintTypeForeignKey: - if c.References == nil { - return FieldRequiredError{Name: "references"} - } - case ConstraintTypeExclude: - if c.Exclude == nil { - return FieldRequiredError{Name: "exclude"} - } - if c.Exclude.Elements == "" { - return FieldRequiredError{Name: "exclude.elements"} - } } } @@ -159,20 +135,14 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { // the new table. func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { columns := make(map[string]*schema.Column, len(o.Columns)) - var primaryKeys []string for _, col := range o.Columns { columns[col.Name] = &schema.Column{ Name: col.Name, } - if col.Pk { - primaryKeys = append(primaryKeys, col.Name) - } } var uniqueConstraints map[string]*schema.UniqueConstraint - var checkConstraints map[string]*schema.CheckConstraint - var foreignKeys map[string]schema.ForeignKey for _, c := range o.Constraints { - switch c.Type { + switch c.Type { //nolint:gocritic // more cases are coming soon case ConstraintTypeUnique: if uniqueConstraints == nil { uniqueConstraints = make(map[string]*schema.UniqueConstraint) @@ -181,35 +151,11 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { Name: c.Name, Columns: c.Columns, } - case ConstraintTypeCheck: - if checkConstraints == nil { - checkConstraints = make(map[string]*schema.CheckConstraint) - } - checkConstraints[c.Name] = &schema.CheckConstraint{ - Name: c.Name, - Columns: c.Columns, - Definition: *c.Check, - } - case ConstraintTypePrimaryKey: - primaryKeys = c.Columns - case ConstraintTypeForeignKey: - if foreignKeys == nil { - foreignKeys = make(map[string]schema.ForeignKey) - } - foreignKeys[c.Name] = schema.ForeignKey{ - Name: c.Name, - Columns: c.Columns, - ReferencedTable: c.References.Table, - ReferencedColumns: c.References.Columns, - OnDelete: string(c.References.OnDelete), - } } } s.AddTable(o.Name, &schema.Table{ Name: o.Name, Columns: columns, - PrimaryKey: primaryKeys, - CheckConstraints: checkConstraints, UniqueConstraints: uniqueConstraints, }) @@ -247,30 +193,35 @@ func constraintsToSQL(constraints []Constraint) (string, error) { writer := &ConstraintSQLWriter{ Name: c.Name, Columns: c.Columns, - IncludeColumns: c.IncludeColumns, - StorageParameters: *c.StorageParameters, - Tablespace: *c.Tablespace, + InitiallyDeferred: c.InitiallyDeferred, + Deferrable: c.Deferrable, + } + if c.IndexParameters != nil { + writer.IncludeColumns = c.IndexParameters.IncludeColumns + if c.IndexParameters.StorageParameters != nil { + writer.StorageParameters = *c.IndexParameters.StorageParameters + } + if c.IndexParameters.Tablespace != nil { + writer.Tablespace = *c.IndexParameters.Tablespace + } } - switch c.Type { - case ConstraintTypeCheck: - constraintsSQL[i] = writer.WriteCheck(*c.Check) - case ConstraintTypeExclude: - constraintsSQL[i] = writer.WriteExclude(c.Exclude.IndexMethod, c.Exclude.Elements) - case ConstraintTypeForeignKey: - constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete, c.References.OnUpdate) - case ConstraintTypePrimaryKey: - constraintsSQL[i] = writer.WritePrimaryKey() + switch c.Type { //nolint:gocritic // more cases are coming soon case ConstraintTypeUnique: - constraintsSQL[i] = writer.WriteUnique(*c.NullsNotDistinct) + constraintsSQL[i] = writer.WriteUnique(c.NullsNotDistinct) } } - return strings.Join(constraintsSQL, ", "), nil + if len(constraintsSQL) == 0 { + return "", nil + } + return ", " + strings.Join(constraintsSQL, ", "), nil } type ConstraintSQLWriter struct { - Name string - Columns []string + Name string + Columns []string + InitiallyDeferred *bool + Deferrable *bool // unique, exclude, primary key constraints support the following options IncludeColumns []string @@ -278,50 +229,14 @@ type ConstraintSQLWriter struct { Tablespace string } -func (w *ConstraintSQLWriter) WriteCheck(check string) string { - constraint := fmt.Sprintf("CONSTRAINT %s CHECK (%s)", pq.QuoteIdentifier(w.Name), check) - return constraint -} - -func (w *ConstraintSQLWriter) WriteExclude(indexMethod, elements string) string { - constraint := fmt.Sprintf("CONSTRAINT %s EXCLUDE USING %s (%s)", pq.QuoteIdentifier(w.Name), indexMethod, elements) - constraint += w.addIndexParameters() - return constraint -} - -func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDeleteAction, onUpdateAction ForeignKeyReferenceOnDelete) string { - onDelete := string(ForeignKeyReferenceOnDeleteNOACTION) - if onDeleteAction != "" { - onDelete = strings.ToUpper(string(onDeleteAction)) - } - onUpdate := string(ForeignKeyReferenceOnDeleteNOACTION) - if onUpdateAction != "" { - onUpdate = strings.ToUpper(string(onUpdateAction)) - } - constraint := fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s ON UPDATE %s", - pq.QuoteIdentifier(w.Name), - strings.Join(quoteColumnNames(w.Columns), ", "), - pq.QuoteIdentifier(referencedTable), - strings.Join(quoteColumnNames(referencedColumns), ", "), - onDelete, - onUpdate, - ) - return constraint -} - -func (w *ConstraintSQLWriter) WritePrimaryKey() string { - constraint := fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", pq.QuoteIdentifier(w.Name), strings.Join(quoteColumnNames(w.Columns), ", ")) - constraint += w.addIndexParameters() - return constraint -} - -func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct bool) string { +func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct *bool) string { nullsDistinct := "" - if nullsNotDistinct { + if nullsNotDistinct != nil && *nullsNotDistinct { nullsDistinct = "NULLS NOT DISTINCT" } constraint := fmt.Sprintf("CONSTRAINT %s UNIQUE %s (%s)", pq.QuoteIdentifier(w.Name), nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) constraint += w.addIndexParameters() + constraint += w.addDeferrable() return constraint } @@ -338,3 +253,25 @@ func (w *ConstraintSQLWriter) addIndexParameters() string { } return constraint } + +func (w *ConstraintSQLWriter) addDeferrable() string { + if w.InitiallyDeferred != nil && *w.Deferrable { + return "" + } + deferrable := "" + if w.Deferrable != nil { + if *w.Deferrable { + deferrable += " DEFERRABLE" + } else { + deferrable += " NOT DEFERRABLE" + } + } + if w.InitiallyDeferred != nil { + if *w.InitiallyDeferred { + deferrable = " INITIALLY DEFERRED" + } else { + deferrable = " INITIALLY IMMEDIATE" + } + } + return deferrable +} diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 8d47d6ef4..947c1df1c 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -483,6 +483,70 @@ func TestCreateTable(t *testing.T) { ColumnMustHaveComment(t, db, schema, "users", "name", "the username") }, }, + { + name: "create table with a unique table constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "name", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "unique_name", + Type: migrations.ConstraintTypeUnique, + Columns: []string{ + "name", + }, + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The unique constraint exists on the new table. + UniqueConstraintMustExist(t, db, schema, "users", "unique_name") + + // Inserting a row into the table succeeds when the unique constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "alice", + }) + + // Inserting a row into the table fails when the unique constraint is not satisfied. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "alice", + }, testutils.UniqueViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the unique constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The check constraint exists on the new table. + UniqueConstraintMustExist(t, db, schema, "users", "unique_name") + + // Inserting a row into the table succeeds when the unique constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "bobby", + }) + + // Inserting a row into the table fails when the unique constraint is not satisfied. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "bobby", + }, testutils.UniqueViolationErrorCode) + }, + }, }) } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 53980eb9f..5332914a7 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -46,20 +46,14 @@ type Column struct { // Constraint definition type Constraint struct { - // Check constraint expression - Check *string `json:"check,omitempty"` - // Columns to add constraint to Columns []string `json:"columns,omitempty"` // Deferable constraint - Deferable *bool `json:"deferable,omitempty"` + Deferrable *bool `json:"deferrable,omitempty"` - // Exclude constraint - Exclude *ConstraintExclude `json:"exclude,omitempty"` - - // IncludeColumns corresponds to the JSON schema field "include_columns". - IncludeColumns []string `json:"include_columns,omitempty"` + // IndexParameters corresponds to the JSON schema field "index_parameters". + IndexParameters *ConstraintIndexParameters `json:"index_parameters,omitempty"` // Initially deferred constraint InitiallyDeferred *bool `json:"initially_deferred,omitempty"` @@ -67,61 +61,26 @@ type Constraint struct { // Name of the constraint Name string `json:"name"` - // No inherit constraint - NoInherit *bool `json:"no_inherit,omitempty"` - // Nulls not distinct constraint NullsNotDistinct *bool `json:"nulls_not_distinct,omitempty"` - // Reference to the foreign key - References *ConstraintReferences `json:"references,omitempty"` - - // StorageParameters corresponds to the JSON schema field "storage_parameters". - StorageParameters *string `json:"storage_parameters,omitempty"` - - // Tablespace corresponds to the JSON schema field "tablespace". - Tablespace *string `json:"tablespace,omitempty"` - // Type of the constraint Type ConstraintType `json:"type"` } -// Exclude constraint -type ConstraintExclude struct { - // Elements corresponds to the JSON schema field "elements". - Elements string `json:"elements"` - - // IndexMethod corresponds to the JSON schema field "index_method". - IndexMethod string `json:"index_method"` - - // Predicate corresponds to the JSON schema field "predicate". - Predicate *string `json:"predicate,omitempty"` -} - -// Reference to the foreign key -type ConstraintReferences struct { - // Columns to reference - Columns []string `json:"columns"` - - // Match type of the foreign key constraint - MatchType *string `json:"match_type,omitempty"` - - // On delete behavior of the foreign key constraint - OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` +type ConstraintIndexParameters struct { + // IncludeColumns corresponds to the JSON schema field "include_columns". + IncludeColumns []string `json:"include_columns,omitempty"` - // On update behavior of the foreign key constraint - OnUpdate ForeignKeyReferenceOnDelete `json:"on_update,omitempty"` + // StorageParameters corresponds to the JSON schema field "storage_parameters". + StorageParameters *string `json:"storage_parameters,omitempty"` - // Name of the table - Table string `json:"table"` + // Tablespace corresponds to the JSON schema field "tablespace". + Tablespace *string `json:"tablespace,omitempty"` } type ConstraintType string -const ConstraintTypeCheck ConstraintType = "check" -const ConstraintTypeExclude ConstraintType = "exclude" -const ConstraintTypeForeignKey ConstraintType = "foreign_key" -const ConstraintTypePrimaryKey ConstraintType = "primary_key" const ConstraintTypeUnique ConstraintType = "unique" // Foreign key reference definition diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index c30f97aaa..7896933f4 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -203,32 +203,11 @@ func convertColumnDef(tableName string, col *pgq.ColumnDef) (*migrations.Column, func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { var constraintType migrations.ConstraintType var nullsNotDistinct *bool - var exclude *migrations.ConstraintExclude switch c.Contype { case pgq.ConstrType_CONSTR_UNIQUE: constraintType = migrations.ConstraintTypeUnique nullsNotDistinct = ptr(c.NullsNotDistinct) - case pgq.ConstrType_CONSTR_EXCLUSION: - exclusionElemens := make([]string, len(c.Exclusions)) - for i, ex := range c.Exclusions { - if len(ex.GetList().Items) != 2 { - return nil, fmt.Errorf("unexpected number of elements in exclusion constraint: %d", len(ex.GetList().Items)) - } - colName := ex.GetList().Items[0].GetIndexElem().Name - opName := ex.GetList().Items[1].GetString_().Sval - exclusionElemens[i] = fmt.Sprintf("%s WITH %s", colName, opName) - } - exclude = &migrations.ConstraintExclude{ - Elements: strings.Join(exclusionElemens, ", "), - IndexMethod: c.AccessMethod, - } - case pgq.ConstrType_CONSTR_PRIMARY: - constraintType = migrations.ConstraintTypePrimaryKey - case pgq.ConstrType_CONSTR_FOREIGN: - constraintType = migrations.ConstraintTypeForeignKey - case pgq.ConstrType_CONSTR_CHECK: - constraintType = migrations.ConstraintTypeCheck default: return nil, fmt.Errorf("unsupported constraint type: %s", c.Contype) } @@ -264,18 +243,22 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { if c.Indexspace != "" { tablespace = ptr(c.Indexspace) } + var indexParameters *migrations.ConstraintIndexParameters + if storageParameters != nil || tablespace != nil || len(including) != 0 { + indexParameters = &migrations.ConstraintIndexParameters{ + StorageParameters: storageParameters, + Tablespace: tablespace, + IncludeColumns: including, + } + } return &migrations.Constraint{ Name: c.Conname, Type: constraintType, NullsNotDistinct: nullsNotDistinct, - Deferable: ptr(c.Deferrable), + Deferrable: ptr(c.Deferrable), InitiallyDeferred: ptr(c.Initdeferred), - NoInherit: ptr(c.IsNoInherit), - StorageParameters: storageParameters, - Tablespace: tablespace, - IncludeColumns: including, - Exclude: exclude, + IndexParameters: indexParameters, }, nil } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index b86b9bbea..c23ca83f8 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -164,6 +164,14 @@ func TestConvertCreateTableStatements(t *testing.T) { sql: "CREATE TABLE foo(a serial PRIMARY KEY, b int DEFAULT 100 CHECK (b > 0), c text NOT NULL UNIQUE)", expectedOp: expect.CreateTableOp21, }, + { + sql: "CREATE TABLE foo(a serial PRIMARY KEY, b text, c text, UNIQUE (b, c)", + expectedOp: expect.CreateTableOp22, + }, + { + sql: "CREATE TABLE foo(b text, c text, UNIQUE (b) include (c) with (fillfactor = 70))", + expectedOp: expect.CreateTableOp23, + }, } for _, tc := range tests { @@ -232,11 +240,9 @@ func TestUnconvertableCreateTableStatements(t *testing.T) { // Table constraints, named and unnamed, are not supported "CREATE TABLE foo(a int, CONSTRAINT foo_check CHECK (a > 0))", - "CREATE TABLE foo(a int, CONSTRAINT foo_unique UNIQUE (a))", "CREATE TABLE foo(a int, CONSTRAINT foo_pk PRIMARY KEY (a))", "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", "CREATE TABLE foo(a int, CHECK (a > 0))", - "CREATE TABLE foo(a int, UNIQUE (a))", "CREATE TABLE foo(a int, PRIMARY KEY (a))", "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b))", diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index dd86fdf69..3ad804a4a 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -294,3 +294,53 @@ var CreateTableOp21 = &migrations.OpCreateTable{ }, }, } + +var CreateTableOp22 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "a", + Type: "serial", + Pk: true, + }, + { + Name: "b", + Type: "text", + }, + { + Name: "c", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeUnique, + Columns: []string{"b", "c"}, + }, + }, +} + +var CreateTableOp23 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "b", + Type: "text", + }, + { + Name: "c", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeUnique, + Columns: []string{"b"}, + IndexParameters: &migrations.ConstraintIndexParameters{ + IncludeColumns: []string{"c"}, + StorageParameters: ptr("fillfactor=70"), + Tablespace: ptr("my_tablespace"), + }, + }, + }, +} diff --git a/schema.json b/schema.json index 510b3453c..3b03effcf 100644 --- a/schema.json +++ b/schema.json @@ -116,46 +116,9 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["unique", "check", "primary_key", "foreign_key", "exclude"] + "enum": ["unique"] }, - "check": { - "description": "Check constraint expression", - "type": "string" - }, - "references": { - "description": "Reference to the foreign key", - "type": "object", - "additionalProperties": false, - "required": ["table", "columns"], - "properties": { - "table": { - "description": "Name of the table", - "type": "string" - }, - "columns": { - "description": "Columns to reference", - "type": "array", - "items": { - "type": "string" - } - }, - "on_delete": { - "description": "On delete behavior of the foreign key constraint", - "$ref": "#/$defs/ForeignKeyReferenceOnDelete", - "default": "NO ACTION" - }, - "on_update": { - "description": "On update behavior of the foreign key constraint", - "$ref": "#/$defs/ForeignKeyReferenceOnDelete", - "default": "NO ACTION" - }, - "match_type": { - "description": "Match type of the foreign key constraint", - "type": "string" - } - } - }, - "deferable": { + "deferrable": { "description": "Deferable constraint", "type": "boolean" }, @@ -163,42 +126,27 @@ "description": "Initially deferred constraint", "type": "boolean" }, - "no_inherit": { - "description": "No inherit constraint", - "type": "boolean" - }, "nulls_not_distinct": { "description": "Nulls not distinct constraint", "type": "boolean" }, - "tablespace": { - "type": "string" - }, - "storage_parameters": { - "type": "string" - }, - "include_columns": { - "type": "array", - "items": { - "type": "string" - } - }, - "exclude": { - "description": "Exclude constraint", - "additionalProperties": false, + "index_parameters": { "type": "object", + "additionalProperties": false, "properties": { - "elements": { + "tablespace": { "type": "string" }, - "index_method": { + "storage_parameters": { "type": "string" }, - "predicate": { - "type": "string" + "include_columns": { + "type": "array", + "items": { + "type": "string" + } } - }, - "required": ["index_method", "elements"] + } } }, "required": ["name", "type"], From 965f28f6d16de2fbb8de4df9f47f19c8787c4891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 15:43:30 +0100 Subject: [PATCH 07/66] minor fixes --- ...50_create_table_with_table_constraint.json | 8 +++-- pkg/sql2pgroll/create_table_test.go | 4 +-- pkg/sql2pgroll/expect/create_table.go | 36 ++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/examples/50_create_table_with_table_constraint.json b/examples/50_create_table_with_table_constraint.json index a90ea6772..08d041f17 100644 --- a/examples/50_create_table_with_table_constraint.json +++ b/examples/50_create_table_with_table_constraint.json @@ -27,9 +27,13 @@ { "name": "unique_numbers", "type": "unique", - "columns": ["phone"], + "columns": [ + "phone" + ], "index_parameters": { - "include_columns": ["name"] + "include_columns": [ + "name" + ] } } ] diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index c23ca83f8..37d4f0e5d 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -165,11 +165,11 @@ func TestConvertCreateTableStatements(t *testing.T) { expectedOp: expect.CreateTableOp21, }, { - sql: "CREATE TABLE foo(a serial PRIMARY KEY, b text, c text, UNIQUE (b, c)", + sql: "CREATE TABLE foo(a serial PRIMARY KEY, b text, c text, UNIQUE (b, c))", expectedOp: expect.CreateTableOp22, }, { - sql: "CREATE TABLE foo(b text, c text, UNIQUE (b) include (c) with (fillfactor = 70))", + sql: "CREATE TABLE foo(b text, c text, UNIQUE (b) include (c) with (fillfactor = 70) using index tablespace my_tablespace)", expectedOp: expect.CreateTableOp23, }, } diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index 3ad804a4a..b5a9b7ac8 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -304,18 +304,23 @@ var CreateTableOp22 = &migrations.OpCreateTable{ Pk: true, }, { - Name: "b", - Type: "text", + Name: "b", + Type: "text", + Nullable: true, }, { - Name: "c", - Type: "text", + Name: "c", + Type: "text", + Nullable: true, }, }, Constraints: []migrations.Constraint{ { - Type: migrations.ConstraintTypeUnique, - Columns: []string{"b", "c"}, + Type: migrations.ConstraintTypeUnique, + Columns: []string{"b", "c"}, + NullsNotDistinct: ptr(false), + Deferrable: ptr(false), + InitiallyDeferred: ptr(false), }, }, } @@ -324,21 +329,26 @@ var CreateTableOp23 = &migrations.OpCreateTable{ Name: "foo", Columns: []migrations.Column{ { - Name: "b", - Type: "text", + Name: "b", + Type: "text", + Nullable: true, }, { - Name: "c", - Type: "text", + Name: "c", + Type: "text", + Nullable: true, }, }, Constraints: []migrations.Constraint{ { - Type: migrations.ConstraintTypeUnique, - Columns: []string{"b"}, + Type: migrations.ConstraintTypeUnique, + Columns: []string{"b"}, + NullsNotDistinct: ptr(false), + Deferrable: ptr(false), + InitiallyDeferred: ptr(false), IndexParameters: &migrations.ConstraintIndexParameters{ IncludeColumns: []string{"c"}, - StorageParameters: ptr("fillfactor=70"), + StorageParameters: ptr("fillfactor = '70'"), Tablespace: ptr("my_tablespace"), }, }, From abd6aed289fc96099a4414e382422f1a76fdd92c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 15:48:54 +0100 Subject: [PATCH 08/66] add missing fixes --- pkg/migrations/op_create_table.go | 6 +++++- pkg/sql2pgroll/create_table.go | 24 ++++++++++++++---------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 60da7ed20..aaecf7f67 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -230,11 +230,15 @@ type ConstraintSQLWriter struct { } func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct *bool) string { + var constraint string + if w.Name != "" { + constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) + } nullsDistinct := "" if nullsNotDistinct != nil && *nullsNotDistinct { nullsDistinct = "NULLS NOT DISTINCT" } - constraint := fmt.Sprintf("CONSTRAINT %s UNIQUE %s (%s)", pq.QuoteIdentifier(w.Name), nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) + constraint += fmt.Sprintf("UNIQUE %s (%s)", nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) constraint += w.addIndexParameters() constraint += w.addDeferrable() return constraint diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 7896933f4..efd67cc45 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -20,9 +20,10 @@ func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { // Convert the table elements - table elements can be: // - Column definitions - // - Table constraints (not supported) + // - Table constraints // - LIKE clauses (not supported) var columns []migrations.Column + var constraints []migrations.Constraint for _, elt := range stmt.TableElts { switch elt.Node.(type) { case *pgq.Node_ColumnDef: @@ -34,20 +35,17 @@ func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { return nil, nil } columns = append(columns, *column) + case *pgq.Node_Constraint: + constraint, err := convertConstraint(elt.GetConstraint()) + if err != nil { + return nil, fmt.Errorf("error converting table constraint: %w", err) + } + constraints = append(constraints, *constraint) default: return nil, nil } } - constraints := make([]migrations.Constraint, len(stmt.Constraints)) - for i, c := range stmt.Constraints { - constraint, err := convertConstraint(c.GetConstraint()) - if err != nil { - return nil, fmt.Errorf("error converting table constraint: %w", err) - } - constraints[i] = *constraint - } - return migrations.Operations{ &migrations.OpCreateTable{ Name: getQualifiedRelationName(stmt.GetRelation()), @@ -212,6 +210,11 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { return nil, fmt.Errorf("unsupported constraint type: %s", c.Contype) } + columns := make([]string, len(c.Keys)) + for i, key := range c.Keys { + columns[i] = key.GetString_().Sval + } + including := make([]string, len(c.Including)) for i, include := range c.Including { including[i] = include.GetString_().Sval @@ -255,6 +258,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { return &migrations.Constraint{ Name: c.Conname, Type: constraintType, + Columns: columns, NullsNotDistinct: nullsNotDistinct, Deferrable: ptr(c.Deferrable), InitiallyDeferred: ptr(c.Initdeferred), From b1dba4a99f1ef6df865e3d7a8daf07ede6fd6011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 15:59:57 +0100 Subject: [PATCH 09/66] moreeee --- docs/operations/create_table.mdx | 29 ++++++++++++++++++++++++++++- pkg/sql2pgroll/create_table.go | 5 ++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index dd559c985..e04817aed 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -9,7 +9,8 @@ description: A create table operation creates a new table in the database. { "create_table": { "name": "name of new table", - "columns": [...] + "columns": [...], + "constraints": [...] } } ``` @@ -40,6 +41,26 @@ Each `column` is defined as: Default values are subject to the usual rules for quoting SQL expressions. In particular, string literals should be surrounded with single quotes. +Each `constraint` is defined as: + +```json +{ + "name": "constraint name", + "type": "constraint type", + "columns": ["list", "of", "columns"], + "nulls_not_distinct": true|false, + "deferrable": true|false, + "initially_deferred": true|false, + "index_parameters": { + "tablespace": "index_tablespace", + "storage_parameters": "parameter=value", + "include_columns": ["list", "of", "columns", "included in index"] + }, +}, +``` + +Supported constraint types: `unique`. + ## Examples ### Create multiple tables @@ -98,3 +119,9 @@ Create a table with a `CHECK` constraint on one column: Create a table with different `DEFAULT` values: + +### Create a table with table level unique constraint + +Create a table with table level constraints: + + \ No newline at end of file diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index efd67cc45..3b78c636d 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -40,6 +40,9 @@ func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { if err != nil { return nil, fmt.Errorf("error converting table constraint: %w", err) } + if constraint == nil { + return nil, nil + } constraints = append(constraints, *constraint) default: return nil, nil @@ -207,7 +210,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { constraintType = migrations.ConstraintTypeUnique nullsNotDistinct = ptr(c.NullsNotDistinct) default: - return nil, fmt.Errorf("unsupported constraint type: %s", c.Contype) + return nil, nil } columns := make([]string, len(c.Keys)) From a0e918caf85a9ab7b4a8573becdab802256b53f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 17:31:25 +0100 Subject: [PATCH 10/66] add default values --- pkg/migrations/op_create_table.go | 42 ++++++++++----------------- pkg/migrations/types.go | 10 +++---- pkg/sql2pgroll/convert.go | 1 + pkg/sql2pgroll/create_table.go | 23 +++++---------- pkg/sql2pgroll/create_table_test.go | 2 ++ pkg/sql2pgroll/expect/create_table.go | 16 +++++----- schema.json | 15 ++++++---- 7 files changed, 50 insertions(+), 59 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index aaecf7f67..7f99aee04 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -195,15 +195,9 @@ func constraintsToSQL(constraints []Constraint) (string, error) { Columns: c.Columns, InitiallyDeferred: c.InitiallyDeferred, Deferrable: c.Deferrable, - } - if c.IndexParameters != nil { - writer.IncludeColumns = c.IndexParameters.IncludeColumns - if c.IndexParameters.StorageParameters != nil { - writer.StorageParameters = *c.IndexParameters.StorageParameters - } - if c.IndexParameters.Tablespace != nil { - writer.Tablespace = *c.IndexParameters.Tablespace - } + IncludeColumns: c.IndexParameters.IncludeColumns, + StorageParameters: c.IndexParameters.StorageParameters, + Tablespace: c.IndexParameters.Tablespace, } switch c.Type { //nolint:gocritic // more cases are coming soon @@ -220,8 +214,8 @@ func constraintsToSQL(constraints []Constraint) (string, error) { type ConstraintSQLWriter struct { Name string Columns []string - InitiallyDeferred *bool - Deferrable *bool + InitiallyDeferred bool + Deferrable bool // unique, exclude, primary key constraints support the following options IncludeColumns []string @@ -229,13 +223,13 @@ type ConstraintSQLWriter struct { Tablespace string } -func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct *bool) string { +func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct bool) string { var constraint string if w.Name != "" { constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) } nullsDistinct := "" - if nullsNotDistinct != nil && *nullsNotDistinct { + if nullsNotDistinct { nullsDistinct = "NULLS NOT DISTINCT" } constraint += fmt.Sprintf("UNIQUE %s (%s)", nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", ")) @@ -259,23 +253,19 @@ func (w *ConstraintSQLWriter) addIndexParameters() string { } func (w *ConstraintSQLWriter) addDeferrable() string { - if w.InitiallyDeferred != nil && *w.Deferrable { + if !w.InitiallyDeferred && !w.Deferrable { return "" } deferrable := "" - if w.Deferrable != nil { - if *w.Deferrable { - deferrable += " DEFERRABLE" - } else { - deferrable += " NOT DEFERRABLE" - } + if w.Deferrable { + deferrable += " DEFERRABLE" + } else { + deferrable += " NOT DEFERRABLE" } - if w.InitiallyDeferred != nil { - if *w.InitiallyDeferred { - deferrable = " INITIALLY DEFERRED" - } else { - deferrable = " INITIALLY IMMEDIATE" - } + if w.InitiallyDeferred { + deferrable += " INITIALLY DEFERRED" + } else { + deferrable += " INITIALLY IMMEDIATE" } return deferrable } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 5332914a7..39d9dca2d 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -50,19 +50,19 @@ type Constraint struct { Columns []string `json:"columns,omitempty"` // Deferable constraint - Deferrable *bool `json:"deferrable,omitempty"` + Deferrable bool `json:"deferrable,omitempty"` // IndexParameters corresponds to the JSON schema field "index_parameters". IndexParameters *ConstraintIndexParameters `json:"index_parameters,omitempty"` // Initially deferred constraint - InitiallyDeferred *bool `json:"initially_deferred,omitempty"` + InitiallyDeferred bool `json:"initially_deferred,omitempty"` // Name of the constraint Name string `json:"name"` // Nulls not distinct constraint - NullsNotDistinct *bool `json:"nulls_not_distinct,omitempty"` + NullsNotDistinct bool `json:"nulls_not_distinct,omitempty"` // Type of the constraint Type ConstraintType `json:"type"` @@ -73,10 +73,10 @@ type ConstraintIndexParameters struct { IncludeColumns []string `json:"include_columns,omitempty"` // StorageParameters corresponds to the JSON schema field "storage_parameters". - StorageParameters *string `json:"storage_parameters,omitempty"` + StorageParameters string `json:"storage_parameters,omitempty"` // Tablespace corresponds to the JSON schema field "tablespace". - Tablespace *string `json:"tablespace,omitempty"` + Tablespace string `json:"tablespace,omitempty"` } type ConstraintType string diff --git a/pkg/sql2pgroll/convert.go b/pkg/sql2pgroll/convert.go index a2e51d57a..5dd2d70e0 100644 --- a/pkg/sql2pgroll/convert.go +++ b/pkg/sql2pgroll/convert.go @@ -20,6 +20,7 @@ func Convert(sql string) (migrations.Operations, error) { } if ops == nil { + fmt.Println("ops is nil") return makeRawSQLOperation(sql), nil } diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 3b78c636d..5e768a4e6 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -15,6 +15,7 @@ import ( func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { // Check if the statement can be converted if !canConvertCreateStatement(stmt) { + fmt.Println("cannot convert create statement") return nil, nil } @@ -203,12 +204,12 @@ func convertColumnDef(tableName string, col *pgq.ColumnDef) (*migrations.Column, func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { var constraintType migrations.ConstraintType - var nullsNotDistinct *bool + var nullsNotDistinct bool switch c.Contype { case pgq.ConstrType_CONSTR_UNIQUE: constraintType = migrations.ConstraintTypeUnique - nullsNotDistinct = ptr(c.NullsNotDistinct) + nullsNotDistinct = c.NullsNotDistinct default: return nil, nil } @@ -240,20 +241,12 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { } options[i] = fmt.Sprintf("%s = '%s'", option.GetDefElem().Defname, val) } - var storageParameters *string - if len(options) != 0 { - storageParameters = ptr(strings.Join(options, ", ")) - } - var tablespace *string - if c.Indexspace != "" { - tablespace = ptr(c.Indexspace) - } var indexParameters *migrations.ConstraintIndexParameters - if storageParameters != nil || tablespace != nil || len(including) != 0 { + if len(options) != 0 || c.Indexspace != "" || len(including) != 0 { indexParameters = &migrations.ConstraintIndexParameters{ - StorageParameters: storageParameters, - Tablespace: tablespace, + StorageParameters: strings.Join(options, ", "), + Tablespace: c.Indexspace, IncludeColumns: including, } } @@ -263,8 +256,8 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { Type: constraintType, Columns: columns, NullsNotDistinct: nullsNotDistinct, - Deferrable: ptr(c.Deferrable), - InitiallyDeferred: ptr(c.Initdeferred), + Deferrable: c.Deferrable, + InitiallyDeferred: c.Initdeferred, IndexParameters: indexParameters, }, nil } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 37d4f0e5d..6bd589faf 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -3,6 +3,7 @@ package sql2pgroll_test import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -181,6 +182,7 @@ func TestConvertCreateTableStatements(t *testing.T) { require.Len(t, ops, 1) + fmt.Println(ops[0]) createTableOp, ok := ops[0].(*migrations.OpCreateTable) require.True(t, ok) diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index b5a9b7ac8..dad8e7fbf 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -318,9 +318,9 @@ var CreateTableOp22 = &migrations.OpCreateTable{ { Type: migrations.ConstraintTypeUnique, Columns: []string{"b", "c"}, - NullsNotDistinct: ptr(false), - Deferrable: ptr(false), - InitiallyDeferred: ptr(false), + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, }, }, } @@ -343,13 +343,13 @@ var CreateTableOp23 = &migrations.OpCreateTable{ { Type: migrations.ConstraintTypeUnique, Columns: []string{"b"}, - NullsNotDistinct: ptr(false), - Deferrable: ptr(false), - InitiallyDeferred: ptr(false), + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, IndexParameters: &migrations.ConstraintIndexParameters{ IncludeColumns: []string{"c"}, - StorageParameters: ptr("fillfactor = '70'"), - Tablespace: ptr("my_tablespace"), + StorageParameters: "fillfactor = '70'", + Tablespace: "my_tablespace", }, }, }, diff --git a/schema.json b/schema.json index 3b03effcf..7ff33f859 100644 --- a/schema.json +++ b/schema.json @@ -120,25 +120,30 @@ }, "deferrable": { "description": "Deferable constraint", - "type": "boolean" + "type": "boolean", + "default": false }, "initially_deferred": { "description": "Initially deferred constraint", - "type": "boolean" + "type": "boolean", + "default": false }, "nulls_not_distinct": { "description": "Nulls not distinct constraint", - "type": "boolean" + "type": "boolean", + "default": false }, "index_parameters": { "type": "object", "additionalProperties": false, "properties": { "tablespace": { - "type": "string" + "type": "string", + "default": "" }, "storage_parameters": { - "type": "string" + "type": "string", + "default": "" }, "include_columns": { "type": "array", From 1ef47fdcd268983ef3fd9eadd2441fd5a8ed7123 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 17:32:00 +0100 Subject: [PATCH 11/66] uppercase --- pkg/sql2pgroll/create_table_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 6bd589faf..20d65e6be 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -170,7 +170,7 @@ func TestConvertCreateTableStatements(t *testing.T) { expectedOp: expect.CreateTableOp22, }, { - sql: "CREATE TABLE foo(b text, c text, UNIQUE (b) include (c) with (fillfactor = 70) using index tablespace my_tablespace)", + sql: "CREATE TABLE foo(b text, c text, UNIQUE (b) INCLUDE (c) WITH (fillfactor = 70) USING INDEX TABLESPACE my_tablespace)", expectedOp: expect.CreateTableOp23, }, } From 253e436dd927da480ce97d39d15d4a8b46c8365f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 10 Jan 2025 17:37:55 +0100 Subject: [PATCH 12/66] add validation test --- pkg/migrations/op_create_table.go | 8 ++++--- pkg/migrations/op_create_table_test.go | 32 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 7f99aee04..a2b85b4ab 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -195,9 +195,11 @@ func constraintsToSQL(constraints []Constraint) (string, error) { Columns: c.Columns, InitiallyDeferred: c.InitiallyDeferred, Deferrable: c.Deferrable, - IncludeColumns: c.IndexParameters.IncludeColumns, - StorageParameters: c.IndexParameters.StorageParameters, - Tablespace: c.IndexParameters.Tablespace, + } + if c.IndexParameters != nil { + writer.IncludeColumns = c.IndexParameters.IncludeColumns + writer.StorageParameters = c.IndexParameters.StorageParameters + writer.Tablespace = c.IndexParameters.Tablespace } switch c.Type { //nolint:gocritic // more cases are coming soon diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 947c1df1c..bab8ce3bb 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -665,6 +665,38 @@ func TestCreateTableValidation(t *testing.T) { }, wantStartErr: migrations.InvalidIdentifierLengthError{Name: invalidName}, }, + { + name: "missing column list in unique constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "table1", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "name", + Type: "varchar(255)", + Unique: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "unique_name", + Type: migrations.ConstraintTypeUnique, + }, + }, + }, + }, + }, + }, + wantStartErr: migrations.FieldRequiredError{Name: "columns"}, + }, }) } From fd54808381de9398bbc5b38a4a42c88ec5739c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 12:46:15 +0100 Subject: [PATCH 13/66] rm messages --- pkg/sql2pgroll/convert.go | 1 - pkg/sql2pgroll/create_table.go | 1 - pkg/sql2pgroll/create_table_test.go | 1 - 3 files changed, 3 deletions(-) diff --git a/pkg/sql2pgroll/convert.go b/pkg/sql2pgroll/convert.go index 5dd2d70e0..a2e51d57a 100644 --- a/pkg/sql2pgroll/convert.go +++ b/pkg/sql2pgroll/convert.go @@ -20,7 +20,6 @@ func Convert(sql string) (migrations.Operations, error) { } if ops == nil { - fmt.Println("ops is nil") return makeRawSQLOperation(sql), nil } diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 5e768a4e6..2dbcdca86 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -15,7 +15,6 @@ import ( func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { // Check if the statement can be converted if !canConvertCreateStatement(stmt) { - fmt.Println("cannot convert create statement") return nil, nil } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 20d65e6be..1c35f9c81 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -182,7 +182,6 @@ func TestConvertCreateTableStatements(t *testing.T) { require.Len(t, ops, 1) - fmt.Println(ops[0]) createTableOp, ok := ops[0].(*migrations.OpCreateTable) require.True(t, ok) From b088ffd2bb8caaf8bf82b31a499d9daa8d7ea91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 12:52:36 +0100 Subject: [PATCH 14/66] deparse storage params --- pkg/sql2pgroll/create_table.go | 27 +++++++++------------------ pkg/sql2pgroll/create_table_test.go | 1 - pkg/sql2pgroll/expect/create_table.go | 2 +- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 2dbcdca86..3e799285d 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -4,7 +4,6 @@ package sql2pgroll import ( "fmt" - "strings" pgq "github.com/xataio/pg_query_go/v6" @@ -223,28 +222,20 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { including[i] = include.GetString_().Sval } - options := make([]string, len(c.Options)) - for i, option := range c.Options { - var val string - switch v := option.GetDefElem().Arg.GetNode().(type) { - case *pgq.Node_Float: - val = v.Float.GetFval() - case *pgq.Node_Integer: - val = fmt.Sprintf("%d", v.Integer.GetIval()) - case *pgq.Node_String_: - val = v.String_.GetSval() - case *pgq.Node_Boolean: - val = v.Boolean.String() - default: - return nil, fmt.Errorf("unsupported storage parameter type: %T", v) + var storageParams string + var err error + if len(c.GetOptions()) > 0 { + storageParams, err = pgq.DeparseRelOptions(c.GetOptions()) + if err != nil { + return nil, fmt.Errorf("parsing options: %w", err) } - options[i] = fmt.Sprintf("%s = '%s'", option.GetDefElem().Defname, val) + storageParams = storageParams[1 : len(storageParams)-1] } var indexParameters *migrations.ConstraintIndexParameters - if len(options) != 0 || c.Indexspace != "" || len(including) != 0 { + if storageParams != "" || c.Indexspace != "" || len(including) != 0 { indexParameters = &migrations.ConstraintIndexParameters{ - StorageParameters: strings.Join(options, ", "), + StorageParameters: storageParams, Tablespace: c.Indexspace, IncludeColumns: including, } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 1c35f9c81..5003a2b9c 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -3,7 +3,6 @@ package sql2pgroll_test import ( - "fmt" "testing" "github.com/stretchr/testify/assert" diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index dad8e7fbf..e821f11d3 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -348,7 +348,7 @@ var CreateTableOp23 = &migrations.OpCreateTable{ InitiallyDeferred: false, IndexParameters: &migrations.ConstraintIndexParameters{ IncludeColumns: []string{"c"}, - StorageParameters: "fillfactor = '70'", + StorageParameters: "fillfactor=70", Tablespace: "my_tablespace", }, }, From e0acc686a2ebbe41a38a82b76ae1013b8011760c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 14:38:04 +0100 Subject: [PATCH 15/66] init check constraints --- pkg/migrations/errors.go | 6 ++ pkg/migrations/op_create_table.go | 54 +++++++++- pkg/migrations/op_create_table_test.go | 139 ++++++++++++++++++++++++- pkg/migrations/types.go | 7 ++ pkg/sql2pgroll/create_table.go | 5 + pkg/sql2pgroll/create_table_test.go | 8 ++ pkg/sql2pgroll/expect/create_table.go | 52 +++++++++ schema.json | 12 ++- 8 files changed, 278 insertions(+), 5 deletions(-) diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index fb803a9a4..2198700ae 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -142,6 +142,12 @@ func (e CheckConstraintError) Unwrap() error { } func (e CheckConstraintError) Error() string { + if e.Column == "" { + return fmt.Sprintf("check constraint in table %q is invalid: %s", + e.Table, + e.Err.Error()) + } + return fmt.Sprintf("check constraint on column %q in table %q is invalid: %s", e.Table, e.Column, diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index a2b85b4ab..e579e64f0 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -116,11 +116,33 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { return fmt.Errorf("invalid constraint: %w", err) } - switch c.Type { //nolint:gocritic // more cases are coming soon + switch c.Type { case ConstraintTypeUnique: if len(c.Columns) == 0 { return FieldRequiredError{Name: "columns"} } + case ConstraintTypeCheck: + if c.Check == "" { + return FieldRequiredError{Name: "check"} + } + if c.Deferrable || c.InitiallyDeferred { + return CheckConstraintError{ + Table: o.Name, + Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE"), + } + } + if c.IndexParameters != nil { + return CheckConstraintError{ + Table: o.Name, + Err: fmt.Errorf("CHECK constraints cannot have index parameters"), + } + } + if c.NullsNotDistinct { + return CheckConstraintError{ + Table: o.Name, + Err: fmt.Errorf("CHECK constraints cannot have NULLS NOT DISTINCT"), + } + } } } @@ -141,8 +163,9 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { } } var uniqueConstraints map[string]*schema.UniqueConstraint + var checkConstraints map[string]*schema.CheckConstraint for _, c := range o.Constraints { - switch c.Type { //nolint:gocritic // more cases are coming soon + switch c.Type { case ConstraintTypeUnique: if uniqueConstraints == nil { uniqueConstraints = make(map[string]*schema.UniqueConstraint) @@ -151,12 +174,22 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { Name: c.Name, Columns: c.Columns, } + case ConstraintTypeCheck: + if checkConstraints == nil { + checkConstraints = make(map[string]*schema.CheckConstraint) + } + checkConstraints[c.Name] = &schema.CheckConstraint{ + Name: c.Name, + Columns: c.Columns, + Definition: c.Check, + } } } s.AddTable(o.Name, &schema.Table{ Name: o.Name, Columns: columns, UniqueConstraints: uniqueConstraints, + CheckConstraints: checkConstraints, }) return s @@ -202,9 +235,11 @@ func constraintsToSQL(constraints []Constraint) (string, error) { writer.Tablespace = c.IndexParameters.Tablespace } - switch c.Type { //nolint:gocritic // more cases are coming soon + switch c.Type { case ConstraintTypeUnique: constraintsSQL[i] = writer.WriteUnique(c.NullsNotDistinct) + case ConstraintTypeCheck: + constraintsSQL[i] = writer.WriteCheck(c.Check, c.NoInherit) } } if len(constraintsSQL) == 0 { @@ -240,6 +275,19 @@ func (w *ConstraintSQLWriter) WriteUnique(nullsNotDistinct bool) string { return constraint } +func (w *ConstraintSQLWriter) WriteCheck(check string, noInherit bool) string { + constraint := "" + if w.Name != "" { + constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) + } + constraint += fmt.Sprintf("CHECK %s", check) + if noInherit { + constraint += " NO INHERIT" + } + constraint += w.addDeferrable() + return constraint +} + func (w *ConstraintSQLWriter) addIndexParameters() string { constraint := "" if len(w.IncludeColumns) != 0 { diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index bab8ce3bb..9a53dd3a3 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -4,6 +4,7 @@ package migrations_test import ( "database/sql" + "fmt" "strings" "testing" @@ -384,7 +385,7 @@ func TestCreateTable(t *testing.T) { }, }, { - name: "create table with a check constraint", + name: "create table with a check constraint on column", migrations: []migrations.Migration{ { Name: "01_create_table", @@ -442,6 +443,108 @@ func TestCreateTable(t *testing.T) { }, testutils.CheckViolationErrorCode) }, }, + { + name: "create table with a table check constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "name", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "check_name_length", + Check: "length(name) > 3", + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The check constraint exists on the new table. + CheckConstraintMustExist(t, db, schema, "users", "check_name_length") + + // Inserting a row into the table succeeds when the check constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "alice", + }) + + // Inserting a row into the table fails when the check constraint is not satisfied. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "b", + }, testutils.CheckViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the check constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The check constraint exists on the new table. + CheckConstraintMustExist(t, db, schema, "users", "check_name_length") + + // Inserting a row into the table succeeds when the check constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "bobby", + }) + + // Inserting a row into the table fails when the check constraint is not satisfied. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "name": "c", + }, testutils.CheckViolationErrorCode) + }, + }, + { + name: "create table with column and table comments", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Comment: ptr("the users table"), + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "name", + Type: "varchar(255)", + Unique: true, + Comment: ptr("the username"), + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The comment has been added to the underlying table. + TableMustHaveComment(t, db, schema, "users", "the users table") + // The comment has been added to the underlying column. + ColumnMustHaveComment(t, db, schema, "users", "name", "the username") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The comment is still present on the underlying table. + TableMustHaveComment(t, db, schema, "users", "the users table") + // The comment is still present on the underlying column. + ColumnMustHaveComment(t, db, schema, "users", "name", "the username") + }, + }, { name: "create table with column and table comments", migrations: []migrations.Migration{ @@ -697,6 +800,40 @@ func TestCreateTableValidation(t *testing.T) { }, wantStartErr: migrations.FieldRequiredError{Name: "columns"}, }, + { + name: "check constraint is not deferrable", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "table1", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "name", + Type: "varchar(255)", + Unique: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "check_name", + Type: migrations.ConstraintTypeCheck, + Check: "length(name) > 0", + Deferrable: true, + }, + }, + }, + }, + }, + }, + wantStartErr: migrations.CheckConstraintError{Table: "table1", Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE")}, + }, }) } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 39d9dca2d..7ba9f04fa 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -46,6 +46,9 @@ type Column struct { // Constraint definition type Constraint struct { + // Check constraint expression + Check string `json:"check,omitempty"` + // Columns to add constraint to Columns []string `json:"columns,omitempty"` @@ -61,6 +64,9 @@ type Constraint struct { // Name of the constraint Name string `json:"name"` + // Do not propagate constraint to child tables + NoInherit bool `json:"no_inherit,omitempty"` + // Nulls not distinct constraint NullsNotDistinct bool `json:"nulls_not_distinct,omitempty"` @@ -81,6 +87,7 @@ type ConstraintIndexParameters struct { type ConstraintType string +const ConstraintTypeCheck ConstraintType = "check" const ConstraintTypeUnique ConstraintType = "unique" // Foreign key reference definition diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 3e799285d..8c83e0deb 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -208,6 +208,8 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { case pgq.ConstrType_CONSTR_UNIQUE: constraintType = migrations.ConstraintTypeUnique nullsNotDistinct = c.NullsNotDistinct + case pgq.ConstrType_CONSTR_CHECK: + constraintType = migrations.ConstraintTypeCheck default: return nil, nil } @@ -241,11 +243,14 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { } } + // TODO deparse CHECK expression + return &migrations.Constraint{ Name: c.Conname, Type: constraintType, Columns: columns, NullsNotDistinct: nullsNotDistinct, + NoInherit: c.IsNoInherit, Deferrable: c.Deferrable, InitiallyDeferred: c.Initdeferred, IndexParameters: indexParameters, diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 5003a2b9c..0e0adf0f5 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -172,6 +172,14 @@ func TestConvertCreateTableStatements(t *testing.T) { sql: "CREATE TABLE foo(b text, c text, UNIQUE (b) INCLUDE (c) WITH (fillfactor = 70) USING INDEX TABLESPACE my_tablespace)", expectedOp: expect.CreateTableOp23, }, + { + sql: "CREATE TABLE foo(b text, c text, CHECK (b=c))", + expectedOp: expect.CreateTableOp24, + }, + { + sql: "CREATE TABLE foo(b text, c text, CHECK (b=c) NO INHERIT INITIALLY IMMEDIATE)", + expectedOp: expect.CreateTableOp25, + }, } for _, tc := range tests { diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index e821f11d3..80eae771a 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -354,3 +354,55 @@ var CreateTableOp23 = &migrations.OpCreateTable{ }, }, } + +var CreateTableOp24 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "b", + Type: "text", + Nullable: true, + }, + { + Name: "c", + Type: "text", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeCheck, + Columns: []string{"b", "c"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + }, + }, +} + +var CreateTableOp25 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "b", + Type: "text", + Nullable: true, + }, + { + Name: "c", + Type: "text", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeCheck, + Columns: []string{"b", "c"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: true, + NoInherit: true, + }, + }, +} diff --git a/schema.json b/schema.json index 7ff33f859..d5a7f7b79 100644 --- a/schema.json +++ b/schema.json @@ -116,7 +116,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["unique"] + "enum": ["unique", "check"] }, "deferrable": { "description": "Deferable constraint", @@ -133,6 +133,16 @@ "type": "boolean", "default": false }, + "no_inherit": { + "description": "Do not propagate constraint to child tables", + "type": "boolean", + "default": false + }, + "check": { + "description": "Check constraint expression", + "type": "string", + "default": "" + }, "index_parameters": { "type": "object", "additionalProperties": false, From dab75c2a8d6a7cd75fb0e2e7afa83dded4aebaa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 16:08:05 +0100 Subject: [PATCH 16/66] add deparsing --- pkg/sql2pgroll/create_table.go | 10 +++++++--- pkg/sql2pgroll/create_table_test.go | 6 ++---- pkg/sql2pgroll/expect/create_table.go | 17 +++++++---------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 8c83e0deb..021900464 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -203,6 +203,8 @@ func convertColumnDef(tableName string, col *pgq.ColumnDef) (*migrations.Column, func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { var constraintType migrations.ConstraintType var nullsNotDistinct bool + var checkExpr string + var err error switch c.Contype { case pgq.ConstrType_CONSTR_UNIQUE: @@ -210,6 +212,10 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { nullsNotDistinct = c.NullsNotDistinct case pgq.ConstrType_CONSTR_CHECK: constraintType = migrations.ConstraintTypeCheck + checkExpr, err = pgq.DeparseExpr(c.GetRawExpr()) + if err != nil { + return nil, fmt.Errorf("deparsing check expression: %w", err) + } default: return nil, nil } @@ -225,7 +231,6 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { } var storageParams string - var err error if len(c.GetOptions()) > 0 { storageParams, err = pgq.DeparseRelOptions(c.GetOptions()) if err != nil { @@ -243,8 +248,6 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { } } - // TODO deparse CHECK expression - return &migrations.Constraint{ Name: c.Conname, Type: constraintType, @@ -254,6 +257,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { Deferrable: c.Deferrable, InitiallyDeferred: c.Initdeferred, IndexParameters: indexParameters, + Check: checkExpr, }, nil } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 0e0adf0f5..0b2ceca8f 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -173,11 +173,11 @@ func TestConvertCreateTableStatements(t *testing.T) { expectedOp: expect.CreateTableOp23, }, { - sql: "CREATE TABLE foo(b text, c text, CHECK (b=c))", + sql: "CREATE TABLE foo(a int, CHECK (a>0))", expectedOp: expect.CreateTableOp24, }, { - sql: "CREATE TABLE foo(b text, c text, CHECK (b=c) NO INHERIT INITIALLY IMMEDIATE)", + sql: "CREATE TABLE foo(b text, c text, CHECK (b=c) NO INHERIT)", expectedOp: expect.CreateTableOp25, }, } @@ -247,10 +247,8 @@ func TestUnconvertableCreateTableStatements(t *testing.T) { "CREATE TABLE foo(a text COLLATE en_US)", // Table constraints, named and unnamed, are not supported - "CREATE TABLE foo(a int, CONSTRAINT foo_check CHECK (a > 0))", "CREATE TABLE foo(a int, CONSTRAINT foo_pk PRIMARY KEY (a))", "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", - "CREATE TABLE foo(a int, CHECK (a > 0))", "CREATE TABLE foo(a int, PRIMARY KEY (a))", "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b))", diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index 80eae771a..6775d1aff 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -359,20 +359,16 @@ var CreateTableOp24 = &migrations.OpCreateTable{ Name: "foo", Columns: []migrations.Column{ { - Name: "b", - Type: "text", - Nullable: true, - }, - { - Name: "c", - Type: "text", + Name: "a", + Type: "int", Nullable: true, }, }, Constraints: []migrations.Constraint{ { Type: migrations.ConstraintTypeCheck, - Columns: []string{"b", "c"}, + Check: "a > 0", + Columns: []string{}, NullsNotDistinct: false, Deferrable: false, InitiallyDeferred: false, @@ -398,10 +394,11 @@ var CreateTableOp25 = &migrations.OpCreateTable{ Constraints: []migrations.Constraint{ { Type: migrations.ConstraintTypeCheck, - Columns: []string{"b", "c"}, + Check: "b = c", + Columns: []string{}, NullsNotDistinct: false, Deferrable: false, - InitiallyDeferred: true, + InitiallyDeferred: false, NoInherit: true, }, }, From cb7b922c971e36b19d2c079be17ca38a103f805d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 16:12:31 +0100 Subject: [PATCH 17/66] add docs --- docs/operations/create_table.mdx | 6 ++++-- examples/50_create_table_with_table_constraint.json | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index e04817aed..b43d4ac9d 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -48,9 +48,11 @@ Each `constraint` is defined as: "name": "constraint name", "type": "constraint type", "columns": ["list", "of", "columns"], + "check": "condition of CHECK constraint", "nulls_not_distinct": true|false, "deferrable": true|false, "initially_deferred": true|false, + "no_inherit": true|false, "index_parameters": { "tablespace": "index_tablespace", "storage_parameters": "parameter=value", @@ -59,7 +61,7 @@ Each `constraint` is defined as: }, ``` -Supported constraint types: `unique`. +Supported constraint types: `unique`, `check`. ## Examples @@ -120,7 +122,7 @@ Create a table with different `DEFAULT` values: -### Create a table with table level unique constraint +### Create a table with multiple table level constraints Create a table with table level constraints: diff --git a/examples/50_create_table_with_table_constraint.json b/examples/50_create_table_with_table_constraint.json index 08d041f17..19edf387b 100644 --- a/examples/50_create_table_with_table_constraint.json +++ b/examples/50_create_table_with_table_constraint.json @@ -35,6 +35,11 @@ "name" ] } + }, + { + "name": "name_must_be_present", + "type": "check", + "check": "length(name) > 0" } ] } From 00bb93f209fd4cdab40aff228a20c9a4d6b6c798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 16:23:27 +0100 Subject: [PATCH 18/66] inimini --- pkg/migrations/errors.go | 4 +++- pkg/migrations/op_create_table.go | 5 ++++- pkg/migrations/op_create_table_test.go | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index 2198700ae..a3b61741b 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -134,6 +134,7 @@ func (e ColumnReferenceError) Error() string { type CheckConstraintError struct { Table string Column string + Name string Err error } @@ -143,7 +144,8 @@ func (e CheckConstraintError) Unwrap() error { func (e CheckConstraintError) Error() string { if e.Column == "" { - return fmt.Sprintf("check constraint in table %q is invalid: %s", + return fmt.Sprintf("check constraint %s in table %q is invalid: %s", + e.Name, e.Table, e.Err.Error()) } diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index e579e64f0..f36162581 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -128,18 +128,21 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { if c.Deferrable || c.InitiallyDeferred { return CheckConstraintError{ Table: o.Name, + Name: c.Name, Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE"), } } if c.IndexParameters != nil { return CheckConstraintError{ Table: o.Name, + Name: c.Name, Err: fmt.Errorf("CHECK constraints cannot have index parameters"), } } if c.NullsNotDistinct { return CheckConstraintError{ Table: o.Name, + Name: c.Name, Err: fmt.Errorf("CHECK constraints cannot have NULLS NOT DISTINCT"), } } @@ -280,7 +283,7 @@ func (w *ConstraintSQLWriter) WriteCheck(check string, noInherit bool) string { if w.Name != "" { constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) } - constraint += fmt.Sprintf("CHECK %s", check) + constraint += fmt.Sprintf("CHECK (%s)", check) if noInherit { constraint += " NO INHERIT" } diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 9a53dd3a3..36d934301 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -832,7 +832,7 @@ func TestCreateTableValidation(t *testing.T) { }, }, }, - wantStartErr: migrations.CheckConstraintError{Table: "table1", Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE")}, + wantStartErr: fmt.Errorf("migration is invalid: %w", migrations.CheckConstraintError{Table: "table1", Name: "check_name", Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE")}), }, }) } From cf25acc778492cb2779805c641ca06b9a56afd07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 13 Jan 2025 16:41:56 +0100 Subject: [PATCH 19/66] even more --- pkg/migrations/errors.go | 2 +- pkg/migrations/op_create_table.go | 3 +-- pkg/migrations/op_create_table_test.go | 12 +++++------- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index a3b61741b..85e121afa 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -144,7 +144,7 @@ func (e CheckConstraintError) Unwrap() error { func (e CheckConstraintError) Error() string { if e.Column == "" { - return fmt.Sprintf("check constraint %s in table %q is invalid: %s", + return fmt.Sprintf("check constraint %q in table %q is invalid: %s", e.Name, e.Table, e.Err.Error()) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index f36162581..dbdcd5958 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -129,7 +129,7 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { return CheckConstraintError{ Table: o.Name, Name: c.Name, - Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE"), + Err: fmt.Errorf("CHECK constraints cannot be marked DEFERRABLE"), } } if c.IndexParameters != nil { @@ -287,7 +287,6 @@ func (w *ConstraintSQLWriter) WriteCheck(check string, noInherit bool) string { if noInherit { constraint += " NO INHERIT" } - constraint += w.addDeferrable() return constraint } diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 36d934301..1cbb2b47c 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -4,7 +4,6 @@ package migrations_test import ( "database/sql" - "fmt" "strings" "testing" @@ -465,6 +464,7 @@ func TestCreateTable(t *testing.T) { Constraints: []migrations.Constraint{ { Name: "check_name_length", + Type: "check", Check: "length(name) > 3", }, }, @@ -801,7 +801,7 @@ func TestCreateTableValidation(t *testing.T) { wantStartErr: migrations.FieldRequiredError{Name: "columns"}, }, { - name: "check constraint is not deferrable", + name: "check constraint missing expression", migrations: []migrations.Migration{ { Name: "01_create_table", @@ -822,17 +822,15 @@ func TestCreateTableValidation(t *testing.T) { }, Constraints: []migrations.Constraint{ { - Name: "check_name", - Type: migrations.ConstraintTypeCheck, - Check: "length(name) > 0", - Deferrable: true, + Name: "check_name", + Type: migrations.ConstraintTypeCheck, }, }, }, }, }, }, - wantStartErr: fmt.Errorf("migration is invalid: %w", migrations.CheckConstraintError{Table: "table1", Name: "check_name", Err: fmt.Errorf("CHECK constraints cannot be marked DEFERABLE")}), + wantStartErr: migrations.FieldRequiredError{Name: "check"}, }, }) } From f33c2ea72efca039a5d3558d81a986a6277f5dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 14 Jan 2025 12:27:43 +0100 Subject: [PATCH 20/66] rm unnecessary test --- pkg/migrations/op_create_table_test.go | 41 -------------------------- 1 file changed, 41 deletions(-) diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 1cbb2b47c..141a14f36 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -545,47 +545,6 @@ func TestCreateTable(t *testing.T) { ColumnMustHaveComment(t, db, schema, "users", "name", "the username") }, }, - { - name: "create table with column and table comments", - migrations: []migrations.Migration{ - { - Name: "01_create_table", - Operations: migrations.Operations{ - &migrations.OpCreateTable{ - Name: "users", - Comment: ptr("the users table"), - Columns: []migrations.Column{ - { - Name: "id", - Type: "serial", - Pk: true, - }, - { - Name: "name", - Type: "varchar(255)", - Unique: true, - Comment: ptr("the username"), - }, - }, - }, - }, - }, - }, - afterStart: func(t *testing.T, db *sql.DB, schema string) { - // The comment has been added to the underlying table. - TableMustHaveComment(t, db, schema, "users", "the users table") - // The comment has been added to the underlying column. - ColumnMustHaveComment(t, db, schema, "users", "name", "the username") - }, - afterRollback: func(t *testing.T, db *sql.DB, schema string) { - }, - afterComplete: func(t *testing.T, db *sql.DB, schema string) { - // The comment is still present on the underlying table. - TableMustHaveComment(t, db, schema, "users", "the users table") - // The comment is still present on the underlying column. - ColumnMustHaveComment(t, db, schema, "users", "name", "the username") - }, - }, { name: "create table with a unique table constraint", migrations: []migrations.Migration{ From 8f9ecf6ecfcd61dc6a80c2fa5b9bab4edf0c4676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 14 Jan 2025 14:10:06 +0100 Subject: [PATCH 21/66] update pr --- go.mod | 2 +- go.sum | 6 ++- internal/jsonschema/jsonschema_test.go | 6 ++- pkg/migrations/op_create_table.go | 10 +---- schema.json | 51 ++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 873d012ce..88653cda5 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/lib/pq v1.10.9 github.com/oapi-codegen/nullable v1.1.0 github.com/pterm/pterm v0.12.80 - github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 + github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 github.com/spf13/cobra v1.8.1 github.com/spf13/viper v1.19.0 github.com/stretchr/testify v1.10.0 diff --git a/go.sum b/go.sum index 33460e00e..0c5412b89 100644 --- a/go.sum +++ b/go.sum @@ -47,6 +47,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/docker/docker v27.1.1+incompatible h1:hO/M4MtV36kzKldqnA37IWhebRA+LnqqcqDja6kVaKY= github.com/docker/docker v27.1.1+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c= @@ -168,8 +170,8 @@ github.com/sagikazarmark/locafero v0.4.0 h1:HApY1R9zGo4DBgr7dqsTH/JJxLTTsOt7u6ke github.com/sagikazarmark/locafero v0.4.0/go.mod h1:Pe1W6UlPYUk/+wc/6KFhbORCfqzgYEpgQ3O5fPuL3H4= github.com/sagikazarmark/slog-shim v0.1.0 h1:diDBnUNK9N/354PgrxMywXnAwEr1QZcOr6gto+ugjYE= github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWRIczQRv+GVI1AkeQ= -github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 h1:lZUw3E0/J3roVtGQ+SCrUrg3ON6NgVqpn3+iol9aGu4= -github.com/santhosh-tekuri/jsonschema/v5 v5.3.1/go.mod h1:uToXkOrWAZ6/Oc07xWQrPOhJotwFIyu2bBVN41fcDUY= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 h1:PKK9DyHxif4LZo+uQSgXNqs0jj5+xZwwfKHgph2lxBw= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.1/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/shirou/gopsutil/v3 v3.24.5 h1:i0t8kL+kQTvpAYToeuiVk3TgDeKOFioZO3Ztz/iZ9pI= diff --git a/internal/jsonschema/jsonschema_test.go b/internal/jsonschema/jsonschema_test.go index d5905d690..616aa6b86 100644 --- a/internal/jsonschema/jsonschema_test.go +++ b/internal/jsonschema/jsonschema_test.go @@ -10,7 +10,7 @@ import ( "strings" "testing" - "github.com/santhosh-tekuri/jsonschema/v5" + "github.com/santhosh-tekuri/jsonschema/v6" "github.com/stretchr/testify/assert" "golang.org/x/tools/txtar" ) @@ -23,7 +23,9 @@ const ( func TestJSONSchemaValidation(t *testing.T) { t.Parallel() - sch := jsonschema.MustCompile(schemaPath) + compiler := jsonschema.NewCompiler() + sch, err := compiler.Compile(schemaPath) + assert.NoError(t, err) files, err := os.ReadDir(testDataDir) assert.NoError(t, err) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index dbdcd5958..3f767c7f1 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -165,22 +165,16 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { Name: col.Name, } } - var uniqueConstraints map[string]*schema.UniqueConstraint - var checkConstraints map[string]*schema.CheckConstraint + uniqueConstraints := make(map[string]*schema.UniqueConstraint, 0) + checkConstraints := make(map[string]*schema.CheckConstraint, 0) for _, c := range o.Constraints { switch c.Type { case ConstraintTypeUnique: - if uniqueConstraints == nil { - uniqueConstraints = make(map[string]*schema.UniqueConstraint) - } uniqueConstraints[c.Name] = &schema.UniqueConstraint{ Name: c.Name, Columns: c.Columns, } case ConstraintTypeCheck: - if checkConstraints == nil { - checkConstraints = make(map[string]*schema.CheckConstraint) - } checkConstraints[c.Name] = &schema.CheckConstraint{ Name: c.Name, Columns: c.Columns, diff --git a/schema.json b/schema.json index d5a7f7b79..11f33e919 100644 --- a/schema.json +++ b/schema.json @@ -164,6 +164,57 @@ } } }, + "allOf": [ + { + "if": { + "properties": { + "type": { + "const": "unique" + } + } + }, + "then": { + "properties": { + "check": { + "const": "", + }, + "no_inherit": { + "const": false + } + }, + "required": ["columns"] + } + }, + { + "if": { + "properties": { + "type": { + "const": "check" + } + } + }, + "then": { + "properties": { + "check": { + "type": "string" + }, + "index_parameters": { + "const": {} + }, + "deferrable": { + "const": false + }, + "initially_deferred": { + "const": false + }, + "nulls_not_distinct": { + "const": false + } + }, + "required": ["check"] + } + } + ], "required": ["name", "type"], "type": "object" }, From f187b8bbcd1461682412fb052a804668c3ea3fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 14 Jan 2025 14:13:41 +0100 Subject: [PATCH 22/66] fmt --- schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schema.json b/schema.json index 11f33e919..54afa1ab2 100644 --- a/schema.json +++ b/schema.json @@ -176,7 +176,7 @@ "then": { "properties": { "check": { - "const": "", + "const": "" }, "no_inherit": { "const": false From 1e527e70679bcb79ae6b02cc1cb2d78af89651a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 14 Jan 2025 14:14:13 +0100 Subject: [PATCH 23/66] add missing updates --- .../create-table-2-check-constraints.txtar | 39 +++++++++++++++++++ ...te-table-3-invalid-check-constraints.txtar | 34 ++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 internal/jsonschema/testdata/create-table-2-check-constraints.txtar create mode 100644 internal/jsonschema/testdata/create-table-3-invalid-check-constraints.txtar diff --git a/internal/jsonschema/testdata/create-table-2-check-constraints.txtar b/internal/jsonschema/testdata/create-table-2-check-constraints.txtar new file mode 100644 index 000000000..bef24b885 --- /dev/null +++ b/internal/jsonschema/testdata/create-table-2-check-constraints.txtar @@ -0,0 +1,39 @@ +This is a valid 'create_table' migration. + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "id", + "type": "serial", + "pk": true + }, + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_check", + "type": "check", + "check": "lenth(title) > 30" + } + ] + } + } + ] +} + +-- valid -- +true \ No newline at end of file diff --git a/internal/jsonschema/testdata/create-table-3-invalid-check-constraints.txtar b/internal/jsonschema/testdata/create-table-3-invalid-check-constraints.txtar new file mode 100644 index 000000000..fa4490e51 --- /dev/null +++ b/internal/jsonschema/testdata/create-table-3-invalid-check-constraints.txtar @@ -0,0 +1,34 @@ +This is an invalid 'create_table' migration. +Check constraint is missing the expression + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_check", + "type": "check" + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file From f292e821a18ded825b27ef53ade889fff4d166f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 14 Jan 2025 15:40:52 +0100 Subject: [PATCH 24/66] initialize primary key constraint --- docs/operations/create_table.mdx | 5 +- ...le-4-invalid-primary-key-constraints.txtar | 38 +++++++ pkg/migrations/errors.go | 8 ++ pkg/migrations/op_common_test.go | 27 +++++ pkg/migrations/op_create_table.go | 39 +++++++- pkg/migrations/op_create_table_test.go | 98 +++++++++++++++++++ pkg/migrations/types.go | 1 + pkg/sql2pgroll/create_table.go | 2 + pkg/sql2pgroll/create_table_test.go | 6 +- pkg/sql2pgroll/expect/create_table.go | 26 +++++ schema.json | 25 ++++- 11 files changed, 269 insertions(+), 6 deletions(-) create mode 100644 internal/jsonschema/testdata/create-table-4-invalid-primary-key-constraints.txtar diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index b43d4ac9d..fa2168974 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -61,7 +61,10 @@ Each `constraint` is defined as: }, ``` -Supported constraint types: `unique`, `check`. +Supported constraint types: `unique`, `check`, `primary_key`. + +Please note that you can only configure primary keys in `columns` list or `constraints` list, but +not in both places. ## Examples diff --git a/internal/jsonschema/testdata/create-table-4-invalid-primary-key-constraints.txtar b/internal/jsonschema/testdata/create-table-4-invalid-primary-key-constraints.txtar new file mode 100644 index 000000000..7ce2cad36 --- /dev/null +++ b/internal/jsonschema/testdata/create-table-4-invalid-primary-key-constraints.txtar @@ -0,0 +1,38 @@ +This is an invalid 'create_table' migration. +Primary key constraint must not constain a check expression. + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_pk", + "type": "primary_key", + "columns": [ + "title" + ], + "check": "this should not be set" + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index 85e121afa..a257a6d5b 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -258,3 +258,11 @@ type MultiColumnConstraintsNotSupportedError struct { func (e MultiColumnConstraintsNotSupportedError) Error() string { return fmt.Sprintf("constraint %q on table %q applies to multiple columns", e.Constraint, e.Table) } + +type PrimaryKeysAreAlreadySetError struct { + Table string +} + +func (e PrimaryKeysAreAlreadySetError) Error() string { + return fmt.Sprintf("table %q already has a primary key configuration in columns list", e.Table) +} diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 5565c9cf1..34788a9bf 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -250,6 +250,13 @@ func NotValidatedForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, co } } +func PrimaryKeyConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) { + t.Helper() + if !primaryKeyConstraintExists(t, db, schema, table, constraint) { + t.Fatalf("Expected constraint %q to exist", constraint) + } +} + func IndexMustExist(t *testing.T, db *sql.DB, schema, table, index string) { t.Helper() if !indexExists(t, db, schema, table, index) { @@ -403,6 +410,26 @@ func foreignKeyExists(t *testing.T, db *sql.DB, schema, table, constraint string return exists } +func primaryKeyConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint string) bool { + t.Helper() + + var exists bool + err := db.QueryRow(` + SELECT EXISTS ( + SELECT 1 + FROM pg_catalog.pg_constraint + WHERE conrelid = $1::regclass + AND conname = $2 + AND contype = 'p' + )`, + fmt.Sprintf("%s.%s", schema, table), constraint).Scan(&exists) + if err != nil { + t.Fatal(err) + } + + return exists +} + func triggerExists(t *testing.T, db *sql.DB, schema, table, trigger string) bool { t.Helper() diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 3f767c7f1..8303f0578 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -79,6 +79,7 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { return TableAlreadyExistsError{Name: o.Name} } + hasPrimaryKeyColumns := false for _, col := range o.Columns { if err := ValidateIdentifierLength(col.Name); err != nil { return fmt.Errorf("invalid column: %w", err) @@ -106,6 +107,10 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { } } } + + if col.Pk { + hasPrimaryKeyColumns = true + } } for _, c := range o.Constraints { @@ -146,6 +151,14 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { Err: fmt.Errorf("CHECK constraints cannot have NULLS NOT DISTINCT"), } } + case ConstraintTypePrimaryKey: + // users can only set primary keys either in columns list or in constraint list + if hasPrimaryKeyColumns { + return PrimaryKeysAreAlreadySetError{Table: o.Name} + } + if len(c.Columns) == 0 { + return FieldRequiredError{Name: "columns"} + } } } @@ -160,10 +173,14 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { // the new table. func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { columns := make(map[string]*schema.Column, len(o.Columns)) + primaryKeys := make([]string, 0) for _, col := range o.Columns { columns[col.Name] = &schema.Column{ Name: col.Name, } + if col.Pk { + primaryKeys = append(primaryKeys, col.Name) + } } uniqueConstraints := make(map[string]*schema.UniqueConstraint, 0) checkConstraints := make(map[string]*schema.CheckConstraint, 0) @@ -180,6 +197,8 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { Columns: c.Columns, Definition: c.Check, } + case ConstraintTypePrimaryKey: + primaryKeys = c.Columns } } s.AddTable(o.Name, &schema.Table{ @@ -187,6 +206,7 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { Columns: columns, UniqueConstraints: uniqueConstraints, CheckConstraints: checkConstraints, + PrimaryKey: primaryKeys, }) return s @@ -207,12 +227,14 @@ func columnsToSQL(cols []Column, tr SQLTransformer) (string, error) { sql += colSQL if col.IsPrimaryKey() { - primaryKeys = append(primaryKeys, pq.QuoteIdentifier(col.Name)) + primaryKeys = append(primaryKeys, col.Name) } } + // Add primary key constraint if there are primary key columns. if len(primaryKeys) > 0 { - sql += fmt.Sprintf(", PRIMARY KEY (%s)", strings.Join(primaryKeys, ", ")) + writer := &ConstraintSQLWriter{Columns: primaryKeys} + sql += ", " + writer.WritePrimaryKey() } return sql, nil } @@ -237,6 +259,8 @@ func constraintsToSQL(constraints []Constraint) (string, error) { constraintsSQL[i] = writer.WriteUnique(c.NullsNotDistinct) case ConstraintTypeCheck: constraintsSQL[i] = writer.WriteCheck(c.Check, c.NoInherit) + case ConstraintTypePrimaryKey: + constraintsSQL[i] = writer.WritePrimaryKey() } } if len(constraintsSQL) == 0 { @@ -284,6 +308,17 @@ func (w *ConstraintSQLWriter) WriteCheck(check string, noInherit bool) string { return constraint } +func (w *ConstraintSQLWriter) WritePrimaryKey() string { + constraint := "" + if w.Name != "" { + constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) + } + constraint += fmt.Sprintf("PRIMARY KEY (%s)", strings.Join(quoteColumnNames(w.Columns), ", ")) + constraint += w.addIndexParameters() + constraint += w.addDeferrable() + return constraint +} + func (w *ConstraintSQLWriter) addIndexParameters() string { constraint := "" if len(w.IncludeColumns) != 0 { diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 141a14f36..77d57be21 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -609,6 +609,71 @@ func TestCreateTable(t *testing.T) { }, testutils.UniqueViolationErrorCode) }, }, + { + name: "create table with primary key constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + }, + { + Name: "name", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "pk_users", + Type: migrations.ConstraintTypePrimaryKey, + Columns: []string{"id"}, + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The check constraint exists on the new table. + PrimaryKeyConstraintMustExist(t, db, schema, "users", "pk_users") + + // Inserting a row into the table succeeds when the PK constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "1", + "name": "alice", + }) + + // Inserting a row into the table fails when the PK constraint is not satisfied. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "1", + "name": "b", + }, testutils.UniqueViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the check constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The check constraint exists on the new table. + PrimaryKeyConstraintMustExist(t, db, schema, "users", "pk_users") + + // Inserting a row into the table succeeds when the PK constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "2", + "name": "bobby", + }) + + // Inserting a row into the table fails when the PK constraint is not satisfied. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "2", + "name": "c", + }, testutils.UniqueViolationErrorCode) + }, + }, }) } @@ -791,6 +856,39 @@ func TestCreateTableValidation(t *testing.T) { }, wantStartErr: migrations.FieldRequiredError{Name: "check"}, }, + { + name: "multiple primary key definitions", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "table1", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: true, + }, + { + Name: "name", + Type: "varchar(255)", + Unique: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "my_pk", + Type: migrations.ConstraintTypePrimaryKey, + Columns: []string{"id"}, + }, + }, + }, + }, + }, + }, + wantStartErr: migrations.PrimaryKeysAreAlreadySetError{Table: "table1"}, + }, }) } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 7ba9f04fa..e0b295a42 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -88,6 +88,7 @@ type ConstraintIndexParameters struct { type ConstraintType string const ConstraintTypeCheck ConstraintType = "check" +const ConstraintTypePrimaryKey ConstraintType = "primary_key" const ConstraintTypeUnique ConstraintType = "unique" // Foreign key reference definition diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 021900464..e65f266c9 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -216,6 +216,8 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { if err != nil { return nil, fmt.Errorf("deparsing check expression: %w", err) } + case pgq.ConstrType_CONSTR_PRIMARY: + constraintType = migrations.ConstraintTypePrimaryKey default: return nil, nil } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 0b2ceca8f..58d70c645 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -180,6 +180,10 @@ func TestConvertCreateTableStatements(t *testing.T) { sql: "CREATE TABLE foo(b text, c text, CHECK (b=c) NO INHERIT)", expectedOp: expect.CreateTableOp25, }, + { + sql: "CREATE TABLE foo(b text, c text, PRIMARY KEY (b) DEFERRABLE)", + expectedOp: expect.CreateTableOp26, + }, } for _, tc := range tests { @@ -247,9 +251,7 @@ func TestUnconvertableCreateTableStatements(t *testing.T) { "CREATE TABLE foo(a text COLLATE en_US)", // Table constraints, named and unnamed, are not supported - "CREATE TABLE foo(a int, CONSTRAINT foo_pk PRIMARY KEY (a))", "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", - "CREATE TABLE foo(a int, PRIMARY KEY (a))", "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b))", // Primary key constraint options are not supported diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index 6775d1aff..3593ecd3e 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -403,3 +403,29 @@ var CreateTableOp25 = &migrations.OpCreateTable{ }, }, } + +var CreateTableOp26 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "b", + Type: "text", + Nullable: true, + }, + { + Name: "c", + Type: "text", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypePrimaryKey, + Columns: []string{"b"}, + NullsNotDistinct: false, + Deferrable: true, + InitiallyDeferred: false, + NoInherit: false, + }, + }, +} diff --git a/schema.json b/schema.json index 54afa1ab2..4e5c43d84 100644 --- a/schema.json +++ b/schema.json @@ -116,7 +116,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["unique", "check"] + "enum": ["unique", "check", "primary_key"] }, "deferrable": { "description": "Deferable constraint", @@ -213,6 +213,29 @@ }, "required": ["check"] } + }, + { + "if": { + "properties": { + "type": { + "const": "primary_key" + } + } + }, + "then": { + "properties": { + "check": { + "const": "" + }, + "no_inherit": { + "const": false + }, + "nulls_not_distinct": { + "const": false + } + }, + "required": ["columns"] + } } ], "required": ["name", "type"], From d788e691d9d604aff8fcdc699a46d03209f7fcb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 13:49:50 +0100 Subject: [PATCH 25/66] add jsonschema tests --- ...primary-key-constraints-extra-check.txtar} | 0 ...mary-key-constraints-missing-columns.txtar | 34 ++++++++++++++++++ ...able-12-valid-primary-key-constraint.txtar | 36 +++++++++++++++++++ 3 files changed, 70 insertions(+) rename internal/jsonschema/testdata/{create-table-4-invalid-primary-key-constraints.txtar => create-table-10-invalid-primary-key-constraints-extra-check.txtar} (100%) create mode 100644 internal/jsonschema/testdata/create-table-11-invalid-primary-key-constraints-missing-columns.txtar create mode 100644 internal/jsonschema/testdata/create-table-12-valid-primary-key-constraint.txtar diff --git a/internal/jsonschema/testdata/create-table-4-invalid-primary-key-constraints.txtar b/internal/jsonschema/testdata/create-table-10-invalid-primary-key-constraints-extra-check.txtar similarity index 100% rename from internal/jsonschema/testdata/create-table-4-invalid-primary-key-constraints.txtar rename to internal/jsonschema/testdata/create-table-10-invalid-primary-key-constraints-extra-check.txtar diff --git a/internal/jsonschema/testdata/create-table-11-invalid-primary-key-constraints-missing-columns.txtar b/internal/jsonschema/testdata/create-table-11-invalid-primary-key-constraints-missing-columns.txtar new file mode 100644 index 000000000..0ea9708b4 --- /dev/null +++ b/internal/jsonschema/testdata/create-table-11-invalid-primary-key-constraints-missing-columns.txtar @@ -0,0 +1,34 @@ +This is an invalid 'create_table' migration. +Primary key constraint must have columns set + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_pk", + "type": "primary_key" + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file diff --git a/internal/jsonschema/testdata/create-table-12-valid-primary-key-constraint.txtar b/internal/jsonschema/testdata/create-table-12-valid-primary-key-constraint.txtar new file mode 100644 index 000000000..628898394 --- /dev/null +++ b/internal/jsonschema/testdata/create-table-12-valid-primary-key-constraint.txtar @@ -0,0 +1,36 @@ +This is a valid 'create_table' migration. + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer" + } + ], + "constraints": [ + { + "name": "my_pk", + "type": "primary_key", + "columns": [ + "title", + "user_id" + ] + } + ] + } + } + ] +} + +-- valid -- +true \ No newline at end of file From 3e542523171b4bec41eea9ab48973c5e4664be81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 13:54:03 +0100 Subject: [PATCH 26/66] rm duplicate --- pkg/sql2pgroll/create_table_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index cf54352af..58d70c645 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -252,7 +252,6 @@ func TestUnconvertableCreateTableStatements(t *testing.T) { // Table constraints, named and unnamed, are not supported "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", - "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b))", // Primary key constraint options are not supported From da600725766ada1a56fa1f511d0646726a0453b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 14:05:33 +0100 Subject: [PATCH 27/66] edit comments --- pkg/migrations/op_create_table_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index bca198e73..942039b9a 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -726,7 +726,7 @@ func TestCreateTable(t *testing.T) { }, }, afterStart: func(t *testing.T, db *sql.DB, schema string) { - // The check constraint exists on the new table. + // The PK constraint exists on the new table. PrimaryKeyConstraintMustExist(t, db, schema, "users", "pk_users") // Inserting a row into the table succeeds when the PK constraint is satisfied. @@ -742,10 +742,10 @@ func TestCreateTable(t *testing.T) { }, testutils.UniqueViolationErrorCode) }, afterRollback: func(t *testing.T, db *sql.DB, schema string) { - // The table has been dropped, so the check constraint is gone. + // The table has been dropped, so the PK constraint is gone. }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { - // The check constraint exists on the new table. + // The PK constraint exists on the new table. PrimaryKeyConstraintMustExist(t, db, schema, "users", "pk_users") // Inserting a row into the table succeeds when the PK constraint is satisfied. From 981df44b2b23e96f860ba9161bf1fe88e309e292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 15:49:21 +0100 Subject: [PATCH 28/66] add support for foreign key constraints --- pkg/migrations/op_create_table.go | 46 ++++++++++++++++++++++ pkg/migrations/types.go | 28 ++++++++++++++ pkg/schema/schema.go | 3 ++ pkg/sql2pgroll/create_table.go | 42 +++++++++++++++++++++ schema.json | 63 ++++++++++++++++++++++++++++++- 5 files changed, 181 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index c830827a4..44570bfab 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -159,6 +159,13 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { if len(c.Columns) == 0 { return FieldRequiredError{Name: "columns"} } + case ConstraintTypeForeignKey: + if len(c.Columns) == 0 { + return FieldRequiredError{Name: "columns"} + } + if c.References == nil { + return FieldRequiredError{Name: "references"} + } } } @@ -186,6 +193,7 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { } uniqueConstraints := make(map[string]*schema.UniqueConstraint, 0) checkConstraints := make(map[string]*schema.CheckConstraint, 0) + foreignKeys := make(map[string]*schema.ForeignKey, 0) for _, c := range o.Constraints { switch c.Type { case ConstraintTypeUnique: @@ -201,6 +209,16 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { } case ConstraintTypePrimaryKey: primaryKeys = c.Columns + case ConstraintTypeForeignKey: + foreignKeys[c.Name] = &schema.ForeignKey{ + Name: c.Name, + Columns: c.Columns, + ReferencedTable: c.References.Table, + ReferencedColumns: c.References.Columns, + OnDelete: string(c.References.OnDelete), + OnUpdate: string(c.References.OnUpdate), + } + } } @@ -210,6 +228,7 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { UniqueConstraints: uniqueConstraints, CheckConstraints: checkConstraints, PrimaryKey: primaryKeys, + ForeignKeys: foreignKeys, }) return s @@ -264,6 +283,8 @@ func constraintsToSQL(constraints []Constraint) (string, error) { constraintsSQL[i] = writer.WriteCheck(c.Check, c.NoInherit) case ConstraintTypePrimaryKey: constraintsSQL[i] = writer.WritePrimaryKey() + case ConstraintTypeForeignKey: + constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete, c.References.OnUpdate) } } if len(constraintsSQL) == 0 { @@ -322,6 +343,31 @@ func (w *ConstraintSQLWriter) WritePrimaryKey() string { return constraint } +func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDelete, onUpdate ForeignKeyReferenceOnDelete) string { + onDeleteAction := string(ForeignKeyReferenceOnDeleteNOACTION) + if onDelete != "" { + onDeleteAction = strings.ToUpper(string(onDeleteAction)) + } + onUpdateAction := string(ForeignKeyReferenceOnDeleteNOACTION) + if onUpdate != "" { + onUpdateAction = strings.ToUpper(string(onUpdateAction)) + } + + constraint := "" + if w.Name != "" { + constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) + } + constraint += fmt.Sprintf("FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s ON UPDATE %s", + strings.Join(quoteColumnNames(w.Columns), ", "), + pq.QuoteIdentifier(referencedTable), + strings.Join(quoteColumnNames(referencedColumns), ", "), + onDeleteAction, + onUpdateAction, + ) + constraint += w.addDeferrable() + return constraint +} + func (w *ConstraintSQLWriter) addIndexParameters() string { constraint := "" if len(w.IncludeColumns) != 0 { diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index e0b295a42..e68ad379f 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -70,6 +70,9 @@ type Constraint struct { // Nulls not distinct constraint NullsNotDistinct bool `json:"nulls_not_distinct,omitempty"` + // Reference to the foreign key + References *ConstraintReferences `json:"references,omitempty"` + // Type of the constraint Type ConstraintType `json:"type"` } @@ -85,9 +88,34 @@ type ConstraintIndexParameters struct { Tablespace string `json:"tablespace,omitempty"` } +// Reference to the foreign key +type ConstraintReferences struct { + // Columns to reference + Columns []string `json:"columns"` + + // Match type of the foreign key constraint + MatchType ConstraintReferencesMatchType `json:"match_type,omitempty"` + + // On delete behavior of the foreign key constraint + OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` + + // On delete behavior of the foreign key constraint + OnUpdate ForeignKeyReferenceOnDelete `json:"on_update,omitempty"` + + // Name of the table + Table string `json:"table"` +} + +type ConstraintReferencesMatchType string + +const ConstraintReferencesMatchTypeFULL ConstraintReferencesMatchType = "FULL" +const ConstraintReferencesMatchTypePARTIAL ConstraintReferencesMatchType = "PARTIAL" +const ConstraintReferencesMatchTypeSIMPLE ConstraintReferencesMatchType = "SIMPLE" + type ConstraintType string const ConstraintTypeCheck ConstraintType = "check" +const ConstraintTypeForeignKey ConstraintType = "foreign_key" const ConstraintTypePrimaryKey ConstraintType = "primary_key" const ConstraintTypeUnique ConstraintType = "unique" diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 790581ad2..6053d3af0 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -125,6 +125,9 @@ type ForeignKey struct { // The ON DELETE behavior of the foreign key OnDelete string `json:"onDelete"` + + // The ON UPDATE behavior of the foreign key + OnUpdate string `json:"onUpdate"` } // CheckConstraint represents a check constraint on a table diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index e65f266c9..006e94eca 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -10,6 +10,14 @@ import ( "github.com/xataio/pgroll/pkg/migrations" ) +var referentialAction = map[string]migrations.ForeignKeyReferenceOnDelete{ + "a": migrations.ForeignKeyReferenceOnDeleteNOACTION, + "c": migrations.ForeignKeyReferenceOnDeleteCASCADE, + "d": migrations.ForeignKeyReferenceOnDeleteSETDEFAULT, + "n": migrations.ForeignKeyReferenceOnDeleteSETNULL, + "r": migrations.ForeignKeyReferenceOnDeleteRESTRICT, +} + // convertCreateStmt converts a CREATE TABLE statement to a pgroll operation. func convertCreateStmt(stmt *pgq.CreateStmt) (migrations.Operations, error) { // Check if the statement can be converted @@ -205,6 +213,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { var nullsNotDistinct bool var checkExpr string var err error + var references *migrations.ConstraintReferences switch c.Contype { case pgq.ConstrType_CONSTR_UNIQUE: @@ -218,6 +227,38 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { } case pgq.ConstrType_CONSTR_PRIMARY: constraintType = migrations.ConstraintTypePrimaryKey + case pgq.ConstrType_CONSTR_FOREIGN: + constraintType = migrations.ConstraintTypeForeignKey + referencedTable := c.Pktable.Relname + referencedColumns := make([]string, len(c.FkAttrs)) + for i, node := range c.PkAttrs { + referencedColumns[i] = node.GetString_().Sval + } + matchType := migrations.ConstraintReferencesMatchTypeSIMPLE + switch c.FkMatchtype { + case "p": + matchType = migrations.ConstraintReferencesMatchTypePARTIAL + case "f": + matchType = migrations.ConstraintReferencesMatchTypeFULL + case "s": + matchType = migrations.ConstraintReferencesMatchTypeSIMPLE + } + onDelete := migrations.ForeignKeyReferenceOnDeleteNOACTION + if c.FkDelAction != "" { + onDelete = referentialAction[c.FkDelAction] + } + onUpdate := migrations.ForeignKeyReferenceOnDeleteNOACTION + if c.FkUpdAction != "" { + onUpdate = referentialAction[c.FkUpdAction] + } + + references = &migrations.ConstraintReferences{ + Table: referencedTable, + Columns: referencedColumns, + MatchType: matchType, + OnDelete: onDelete, + OnUpdate: onUpdate, + } default: return nil, nil } @@ -260,6 +301,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { InitiallyDeferred: c.Initdeferred, IndexParameters: indexParameters, Check: checkExpr, + References: references, }, nil } diff --git a/schema.json b/schema.json index 4e5c43d84..cdec5bb9c 100644 --- a/schema.json +++ b/schema.json @@ -116,7 +116,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["unique", "check", "primary_key"] + "enum": ["unique", "check", "primary_key", "foreign_key"] }, "deferrable": { "description": "Deferable constraint", @@ -143,6 +143,41 @@ "type": "string", "default": "" }, + "references": { + "description": "Reference to the foreign key", + "type": "object", + "additionalProperties": false, + "required": ["table", "columns"], + "properties": { + "table": { + "description": "Name of the table", + "type": "string" + }, + "columns": { + "description": "Columns to reference", + "type": "array", + "items": { + "type": "string" + } + }, + "match_type": { + "description": "Match type of the foreign key constraint", + "type": "string", + "enum": ["SIMPLE", "FULL", "PARTIAL"], + "default": "SIMPLE" + }, + "on_update": { + "description": "On delete behavior of the foreign key constraint", + "$ref": "#/$defs/ForeignKeyReferenceOnDelete", + "default": "NO ACTION" + }, + "on_delete": { + "description": "On delete behavior of the foreign key constraint", + "$ref": "#/$defs/ForeignKeyReferenceOnDelete", + "default": "NO ACTION" + } + } + }, "index_parameters": { "type": "object", "additionalProperties": false, @@ -236,6 +271,32 @@ }, "required": ["columns"] } + }, + { + "if": { + "properties": { + "type": { + "const": "foreign_key" + } + } + }, + "then": { + "properties": { + "check": { + "const": "" + }, + "index_parameters": { + "const": {} + }, + "no_inherit": { + "const": false + }, + "nulls_not_distinct": { + "const": false + } + }, + "required": ["columns", "references"] + } } ], "required": ["name", "type"], From 40dc814bf89e184897c5e1412cfdd3bd37e4720b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 16:11:27 +0100 Subject: [PATCH 29/66] add tests --- docs/operations/create_table.mdx | 10 ++++- pkg/sql2pgroll/create_table.go | 16 +++++--- pkg/sql2pgroll/create_table_test.go | 12 ++++-- pkg/sql2pgroll/expect/create_table.go | 57 +++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 11 deletions(-) diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index fa2168974..7ff5bb32f 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -53,6 +53,14 @@ Each `constraint` is defined as: "deferrable": true|false, "initially_deferred": true|false, "no_inherit": true|false, + "references": { + "name": "name of foreign key constraint", + "table": "name of referenced table", + "columns": ["list", "of", "referenced", "columns"], + "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", + "on_update": "ON UPDATE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", + "match_type": "match type, can be SIMPLE, FULL or PARTIAL. Default is SIMPLE" + }, "index_parameters": { "tablespace": "index_tablespace", "storage_parameters": "parameter=value", @@ -61,7 +69,7 @@ Each `constraint` is defined as: }, ``` -Supported constraint types: `unique`, `check`, `primary_key`. +Supported constraint types: `unique`, `check`, `primary_key`, `foreign_key`. Please note that you can only configure primary keys in `columns` list or `constraints` list, but not in both places. diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index 006e94eca..e948d8e04 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -215,6 +215,11 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { var err error var references *migrations.ConstraintReferences + columns := make([]string, len(c.Keys)) + for i, key := range c.Keys { + columns[i] = key.GetString_().Sval + } + switch c.Contype { case pgq.ConstrType_CONSTR_UNIQUE: constraintType = migrations.ConstraintTypeUnique @@ -230,7 +235,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { case pgq.ConstrType_CONSTR_FOREIGN: constraintType = migrations.ConstraintTypeForeignKey referencedTable := c.Pktable.Relname - referencedColumns := make([]string, len(c.FkAttrs)) + referencedColumns := make([]string, len(c.PkAttrs)) for i, node := range c.PkAttrs { referencedColumns[i] = node.GetString_().Sval } @@ -251,6 +256,10 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { if c.FkUpdAction != "" { onUpdate = referentialAction[c.FkUpdAction] } + columns = make([]string, len(c.FkAttrs)) + for i, node := range c.FkAttrs { + columns[i] = node.GetString_().Sval + } references = &migrations.ConstraintReferences{ Table: referencedTable, @@ -263,11 +272,6 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { return nil, nil } - columns := make([]string, len(c.Keys)) - for i, key := range c.Keys { - columns[i] = key.GetString_().Sval - } - including := make([]string, len(c.Including)) for i, include := range c.Including { including[i] = include.GetString_().Sval diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 58d70c645..87a123862 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -184,6 +184,14 @@ func TestConvertCreateTableStatements(t *testing.T) { sql: "CREATE TABLE foo(b text, c text, PRIMARY KEY (b) DEFERRABLE)", expectedOp: expect.CreateTableOp26, }, + { + sql: "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", + expectedOp: expect.CreateTableOp27, + }, + { + sql: "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b) ON DELETE SET NULL ON UPDATE CASCADE)", + expectedOp: expect.CreateTableOp28, + }, } for _, tc := range tests { @@ -250,10 +258,6 @@ func TestUnconvertableCreateTableStatements(t *testing.T) { // Column collation is not supported "CREATE TABLE foo(a text COLLATE en_US)", - // Table constraints, named and unnamed, are not supported - "CREATE TABLE foo(a int, CONSTRAINT foo_fk FOREIGN KEY (a) REFERENCES bar(b))", - "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b))", - // Primary key constraint options are not supported "CREATE TABLE foo(a int PRIMARY KEY USING INDEX TABLESPACE bar)", "CREATE TABLE foo(a int PRIMARY KEY WITH (fillfactor=70))", diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index 3593ecd3e..34d66768a 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -429,3 +429,60 @@ var CreateTableOp26 = &migrations.OpCreateTable{ }, }, } + +var CreateTableOp27 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "a", + Type: "int", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeForeignKey, + Name: "foo_fk", + Columns: []string{"a"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + References: &migrations.ConstraintReferences{ + Table: "bar", + Columns: []string{"b"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteNOACTION, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteNOACTION, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + }, + }, + }, +} + +var CreateTableOp28 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "a", + Type: "int", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeForeignKey, + Columns: []string{"a"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + References: &migrations.ConstraintReferences{ + Table: "bar", + Columns: []string{"b"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + }, + }, + }, +} From cbf5043cc1562f4a4c73d5c9033c6c638a564c82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 16:31:47 +0100 Subject: [PATCH 30/66] add e2e --- pkg/migrations/op_create_table_test.go | 99 ++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 942039b9a..082e75159 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -761,6 +761,105 @@ func TestCreateTable(t *testing.T) { }, testutils.UniqueViolationErrorCode) }, }, + { + name: "create table with foreign key constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_referenced_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "owners", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + }, + { + Name: "name", + Type: "text", + }, + { + Name: "city", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "pk_owners", + Type: migrations.ConstraintTypePrimaryKey, + Columns: []string{"id"}, + }, + }, + }, + }, + }, + { + Name: "02_create_referencing_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "pets", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + }, + { + Name: "owner_id", + Type: "int", + }, + { + Name: "name", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "fk_users", + Type: migrations.ConstraintTypeForeignKey, + Columns: []string{"owner_id"}, + References: migrations.ConstraintReferences{ + Table: "owners", + Columns: []string{"id"}, + OnDelete: migrations.ReferenceOptionCascade, + OnUpdate: migrations.ReferenceOptionCascade, + }, + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The PK constraint exists on the new table. + PrimaryKeyConstraintMustExist(t, db, schema, "owners", "pk_owners") + ValidatedForeignKeyMustExist(t, db, schema, "pets", "fk_owners") + + MustInsert(t, db, schema, "01_create_referenced_table", "owners", map[string]string{ + "id": "1", + "name": "alice", + "city": "new york", + }) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the FK constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + ValidatedForeignKeyMustExist(t, db, schema, "pets", "fk_owners") + + // Inserting a row into the table succeeds when the PK constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "2", + "name": "bobby", + }) + + // Inserting a row into the table fails when the PK constraint is not satisfied. + MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "2", + "owner_id": "1", + "name": "cutie pie", + }) + }, + }, }) } From b63d0e062ae5f4f1b53c522af8d3705cc04a75d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Thu, 16 Jan 2025 17:07:21 +0100 Subject: [PATCH 31/66] update test --- pkg/migrations/op_create_table.go | 5 ++-- pkg/migrations/op_create_table_test.go | 34 ++++++++++++++------------ 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 44570bfab..702a744fa 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -26,6 +26,7 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri if err != nil { return nil, fmt.Errorf("failed to create constraints SQL: %w", err) } + fmt.Println("constraintsSQL", constraintsSQL) // Create the table _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s %s)", @@ -346,11 +347,11 @@ func (w *ConstraintSQLWriter) WritePrimaryKey() string { func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDelete, onUpdate ForeignKeyReferenceOnDelete) string { onDeleteAction := string(ForeignKeyReferenceOnDeleteNOACTION) if onDelete != "" { - onDeleteAction = strings.ToUpper(string(onDeleteAction)) + onDeleteAction = strings.ToUpper(string(onDelete)) } onUpdateAction := string(ForeignKeyReferenceOnDeleteNOACTION) if onUpdate != "" { - onUpdateAction = strings.ToUpper(string(onUpdateAction)) + onUpdateAction = strings.ToUpper(string(onUpdate)) } constraint := "" diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 082e75159..29a034c24 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -814,14 +814,13 @@ func TestCreateTable(t *testing.T) { }, Constraints: []migrations.Constraint{ { - Name: "fk_users", + Name: "fk_owners", Type: migrations.ConstraintTypeForeignKey, Columns: []string{"owner_id"}, - References: migrations.ConstraintReferences{ + References: &migrations.ConstraintReferences{ Table: "owners", Columns: []string{"id"}, - OnDelete: migrations.ReferenceOptionCascade, - OnUpdate: migrations.ReferenceOptionCascade, + OnDelete: migrations.ForeignKeyReferenceOnDeleteCASCADE, }, }, }, @@ -830,10 +829,6 @@ func TestCreateTable(t *testing.T) { }, }, afterStart: func(t *testing.T, db *sql.DB, schema string) { - // The PK constraint exists on the new table. - PrimaryKeyConstraintMustExist(t, db, schema, "owners", "pk_owners") - ValidatedForeignKeyMustExist(t, db, schema, "pets", "fk_owners") - MustInsert(t, db, schema, "01_create_referenced_table", "owners", map[string]string{ "id": "1", "name": "alice", @@ -844,20 +839,27 @@ func TestCreateTable(t *testing.T) { // The table has been dropped, so the FK constraint is gone. }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { - ValidatedForeignKeyMustExist(t, db, schema, "pets", "fk_owners") - // Inserting a row into the table succeeds when the PK constraint is satisfied. - MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ - "id": "2", - "name": "bobby", + MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "1", + "owner_id": "1", + "name": "cutie pie", }) // Inserting a row into the table fails when the PK constraint is not satisfied. - MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + MustNotInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ "id": "2", - "owner_id": "1", - "name": "cutie pie", + "owner_id": "0", + "name": "bobby", + }, testutils.FKViolationErrorCode) + + // Deleting the row from the referenced table cascades to the referencing table. + MustDelete(t, db, schema, "02_create_referencing_table", "owners", map[string]string{ + "id": "1", }) + + rows := MustSelect(t, db, schema, "02_create_referencing_table", "pets") + assert.Equal(t, []map[string]any{}, rows) }, }, }) From f0913a3cd532cb18fedcd5a7d5959837506a8869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 17 Jan 2025 17:14:21 +0100 Subject: [PATCH 32/66] add example --- docs/operations/create_table.mdx | 8 +- examples/.ledger | 1 + ...50_create_table_with_table_constraint.json | 19 ++--- ...ble_with_table_foreign_key_constraint.json | 75 +++++++++++++++++++ pkg/migrations/op_create_table.go | 1 - 5 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 examples/51_create_table_with_table_foreign_key_constraint.json diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index 7ff5bb32f..2a8a8d7a1 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -137,4 +137,10 @@ Create a table with different `DEFAULT` values: Create a table with table level constraints: - \ No newline at end of file + + +### Create a table with table level foreign key constraints + +Create a table with table level foreign key constraints: + + \ No newline at end of file diff --git a/examples/.ledger b/examples/.ledger index 9f415628e..7003ffe5b 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -48,3 +48,4 @@ 48_drop_tickets_check.json 49_unset_not_null_on_indexed_column.json 50_create_table_with_table_constraint.json +51_create_table_with_table_foreign_key_constraint.json diff --git a/examples/50_create_table_with_table_constraint.json b/examples/50_create_table_with_table_constraint.json index 6e51f7bbc..061ee83ae 100644 --- a/examples/50_create_table_with_table_constraint.json +++ b/examples/50_create_table_with_table_constraint.json @@ -3,7 +3,7 @@ "operations": [ { "create_table": { - "name": "phonebook", + "name": "telephone_providers", "columns": [ { "name": "id", @@ -14,33 +14,28 @@ "type": "varchar(255)" }, { - "name": "city", + "name": "tax_id", "type": "varchar(255)" }, { - "name": "phone", + "name": "headquarters", "type": "varchar(255)" } ], "constraints": [ { - "name": "phonebook_pk", + "name": "provider_pk", "type": "primary_key", "columns": [ "id" ] }, { - "name": "unique_numbers", + "name": "unique_tax_id", "type": "unique", "columns": [ - "phone" - ], - "index_parameters": { - "include_columns": [ - "name" - ] - } + "tax_id" + ] }, { "name": "name_must_be_present", diff --git a/examples/51_create_table_with_table_foreign_key_constraint.json b/examples/51_create_table_with_table_foreign_key_constraint.json new file mode 100644 index 000000000..044c78ae7 --- /dev/null +++ b/examples/51_create_table_with_table_foreign_key_constraint.json @@ -0,0 +1,75 @@ +{ + "name": "51_create_table_with_table_foreign_key_constraint", + "operations": [ + { + "create_table": { + "name": "phonebook", + "columns": [ + { + "name": "id", + "type": "serial" + }, + { + "name": "provider_id", + "type": "serial" + }, + { + "name": "name", + "type": "varchar(255)" + }, + { + "name": "city", + "type": "varchar(255)" + }, + { + "name": "phone", + "type": "varchar(255)" + } + ], + "constraints": [ + { + "name": "phonebook_pk", + "type": "primary_key", + "columns": [ + "id" + ] + }, + { + "name": "provider_fk", + "type": "foreign_key", + "columns": [ + "provider_id" + ], + "deferrable": false, + "references": { + "table": "provider", + "columns": [ + "id" + ], + "on_delete": "CASCADE", + "on_update": "CASCADE", + "match": "SIMPLE" + } + }, + { + "name": "unique_numbers", + "type": "unique", + "columns": [ + "phone" + ], + "index_parameters": { + "include_columns": [ + "name" + ] + } + }, + { + "name": "name_must_be_present", + "type": "check", + "check": "length(name) > 0" + } + ] + } + } + ] +} diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index fd4e29383..6b8fddd6b 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -26,7 +26,6 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri if err != nil { return nil, fmt.Errorf("failed to create constraints SQL: %w", err) } - fmt.Println("constraintsSQL", constraintsSQL) // Create the table _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s %s)", From c82f830639a6414a9d63f0e8f9d9e3a1883b2f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 17 Jan 2025 17:17:34 +0100 Subject: [PATCH 33/66] minor fix --- examples/51_create_table_with_table_foreign_key_constraint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/51_create_table_with_table_foreign_key_constraint.json b/examples/51_create_table_with_table_foreign_key_constraint.json index 044c78ae7..f76cbe9a2 100644 --- a/examples/51_create_table_with_table_foreign_key_constraint.json +++ b/examples/51_create_table_with_table_foreign_key_constraint.json @@ -48,7 +48,7 @@ ], "on_delete": "CASCADE", "on_update": "CASCADE", - "match": "SIMPLE" + "match_type": "SIMPLE" } }, { From 385e7d5d52cc2e5590c6c5d1f1fb00e0ec20f7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 17 Jan 2025 17:28:33 +0100 Subject: [PATCH 34/66] add jsonschema tests --- ...ble-13-invalid-fk-missing-references.txtar | 35 ++++++++++++++++ ...-invalid-fk-missing-referenced-table.txtar | 40 +++++++++++++++++++ ...-table-15-invalid-fk-missing-columns.txtar | 35 ++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 internal/jsonschema/testdata/create-table-13-invalid-fk-missing-references.txtar create mode 100644 internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar create mode 100644 internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar diff --git a/internal/jsonschema/testdata/create-table-13-invalid-fk-missing-references.txtar b/internal/jsonschema/testdata/create-table-13-invalid-fk-missing-references.txtar new file mode 100644 index 000000000..88b661c3c --- /dev/null +++ b/internal/jsonschema/testdata/create-table-13-invalid-fk-missing-references.txtar @@ -0,0 +1,35 @@ +This is an invalid 'create_table' migration. +Foreign key constraints must have references configured + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_fk", + "type": "foreign_key", + "columns": ["title"] + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file diff --git a/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar b/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar new file mode 100644 index 000000000..1c6a4a1e0 --- /dev/null +++ b/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar @@ -0,0 +1,40 @@ +This is an invalid 'create_table' migration. +Foreign key constraints must have referenced table configured + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_fk", + "type": "foreign_key", + "columns": ["title"], + "refereces": { + "columns": [ + "referenced" + ] + } + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file diff --git a/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar b/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar new file mode 100644 index 000000000..88b661c3c --- /dev/null +++ b/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar @@ -0,0 +1,35 @@ +This is an invalid 'create_table' migration. +Foreign key constraints must have references configured + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_fk", + "type": "foreign_key", + "columns": ["title"] + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file From 37393e9ba238dfb5675ff3419bbbf9de04533211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 17 Jan 2025 17:30:48 +0100 Subject: [PATCH 35/66] update test --- .../create-table-15-invalid-fk-missing-columns.txtar | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar b/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar index 88b661c3c..2cfbb9296 100644 --- a/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar +++ b/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar @@ -23,7 +23,12 @@ Foreign key constraints must have references configured { "name": "my_invalid_fk", "type": "foreign_key", - "columns": ["title"] + "references": { + "columns": ["title"], + "table": "referenced", + "on_delete": "CASCADE", + "on_update": "SET NULL" + } } ] } From be2830c2605c96bbf9c682deca563c373b77945f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 17 Jan 2025 17:34:05 +0100 Subject: [PATCH 36/66] fix table name --- examples/51_create_table_with_table_foreign_key_constraint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/51_create_table_with_table_foreign_key_constraint.json b/examples/51_create_table_with_table_foreign_key_constraint.json index f76cbe9a2..288dc15aa 100644 --- a/examples/51_create_table_with_table_foreign_key_constraint.json +++ b/examples/51_create_table_with_table_foreign_key_constraint.json @@ -42,7 +42,7 @@ ], "deferrable": false, "references": { - "table": "provider", + "table": "telephone_providers", "columns": [ "id" ], From 1e0d40385bb83ffa4673e128867722d4ed70cc7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 20 Jan 2025 12:13:30 +0100 Subject: [PATCH 37/66] pass match_type to schema.foreignkey --- pkg/migrations/op_create_table.go | 1 + pkg/schema/schema.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 6b8fddd6b..02c500240 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -217,6 +217,7 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { ReferencedColumns: c.References.Columns, OnDelete: string(c.References.OnDelete), OnUpdate: string(c.References.OnUpdate), + MatchType: string(c.References.MatchType), } } } diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 6053d3af0..e9a1daae1 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -128,6 +128,9 @@ type ForeignKey struct { // The ON UPDATE behavior of the foreign key OnUpdate string `json:"onUpdate"` + + // MatchType is the match type of the foreign key + MatchType string `json:"match_type"` } // CheckConstraint represents a check constraint on a table From d6f9cf7a341061debb2e5d41bb0e7065bc28e1ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 18:20:16 +0100 Subject: [PATCH 38/66] Address review notes and add new attribute --- ...-invalid-fk-missing-referenced-table.txtar | 4 +- ...-table-15-invalid-fk-missing-columns.txtar | 2 +- pkg/migrations/types.go | 5 +- pkg/sql2pgroll/create_table.go | 15 ++- pkg/sql2pgroll/create_table_test.go | 12 ++ pkg/sql2pgroll/expect/create_table.go | 115 ++++++++++++++++++ schema.json | 9 +- 7 files changed, 152 insertions(+), 10 deletions(-) diff --git a/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar b/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar index 1c6a4a1e0..786a51a7e 100644 --- a/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar +++ b/internal/jsonschema/testdata/create-table-14-invalid-fk-missing-referenced-table.txtar @@ -1,5 +1,5 @@ This is an invalid 'create_table' migration. -Foreign key constraints must have referenced table configured +Foreign key constraints must have referenced table configured in references.table -- create_table.json -- { @@ -24,7 +24,7 @@ Foreign key constraints must have referenced table configured "name": "my_invalid_fk", "type": "foreign_key", "columns": ["title"], - "refereces": { + "references": { "columns": [ "referenced" ] diff --git a/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar b/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar index 2cfbb9296..7c0b2e46b 100644 --- a/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar +++ b/internal/jsonschema/testdata/create-table-15-invalid-fk-missing-columns.txtar @@ -1,5 +1,5 @@ This is an invalid 'create_table' migration. -Foreign key constraints must have references configured +Foreign key constraints must have columns configured -- create_table.json -- { diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 8d2def042..a89f4cefe 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -99,7 +99,10 @@ type ConstraintReferences struct { // On delete behavior of the foreign key constraint OnDelete ForeignKeyReferenceOnDelete `json:"on_delete,omitempty"` - // On delete behavior of the foreign key constraint + // Columns to set to null or to default on delete + OnDeleteSetColumns []string `json:"on_delete_set_columns,omitempty"` + + // On update behavior of the foreign key constraint OnUpdate ForeignKeyReferenceOnDelete `json:"on_update,omitempty"` // Name of the table diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index e948d8e04..5ecd02bc3 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -248,9 +248,13 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { case "s": matchType = migrations.ConstraintReferencesMatchTypeSIMPLE } + columnsToSet := make([]string, len(c.FkDelSetCols)) onDelete := migrations.ForeignKeyReferenceOnDeleteNOACTION if c.FkDelAction != "" { onDelete = referentialAction[c.FkDelAction] + for i, node := range c.FkDelSetCols { + columnsToSet[i] = node.GetString_().Sval + } } onUpdate := migrations.ForeignKeyReferenceOnDeleteNOACTION if c.FkUpdAction != "" { @@ -262,11 +266,12 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { } references = &migrations.ConstraintReferences{ - Table: referencedTable, - Columns: referencedColumns, - MatchType: matchType, - OnDelete: onDelete, - OnUpdate: onUpdate, + Table: referencedTable, + Columns: referencedColumns, + MatchType: matchType, + OnDelete: onDelete, + OnDeleteSetColumns: columnsToSet, + OnUpdate: onUpdate, } default: return nil, nil diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 87a123862..6f84c48a5 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -192,6 +192,18 @@ func TestConvertCreateTableStatements(t *testing.T) { sql: "CREATE TABLE foo(a int, FOREIGN KEY (a) REFERENCES bar(b) ON DELETE SET NULL ON UPDATE CASCADE)", expectedOp: expect.CreateTableOp28, }, + { + sql: "CREATE TABLE foo(a int, b int, FOREIGN KEY (a, b) REFERENCES bar(c, d) ON DELETE SET NULL (b))", + expectedOp: expect.CreateTableOp29, + }, + { + sql: "CREATE TABLE foo(a int, b int, c int, FOREIGN KEY (a, b, c) REFERENCES bar(d, e, f) ON DELETE SET NULL ON UPDATE CASCADE)", + expectedOp: expect.CreateTableOp30, + }, + { + sql: "CREATE TABLE foo(a int, b int, c int, FOREIGN KEY (a, b, c) REFERENCES bar(d, e, f) MATCH FULL)", + expectedOp: expect.CreateTableOp31, + }, } for _, tc := range tests { diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index 34d66768a..d88877ba1 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -486,3 +486,118 @@ var CreateTableOp28 = &migrations.OpCreateTable{ }, }, } + +var CreateTableOp29 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "a", + Type: "int", + Nullable: true, + }, + { + Name: "b", + Type: "int", + Nullable: true, + }, + { + Name: "c", + Type: "int", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeForeignKey, + Columns: []string{"a", "b", "c"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + References: &migrations.ConstraintReferences{ + Table: "bar", + Columns: []string{"b", "c", "d"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnDeleteSetColumns: []string{"b"}, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteSETNULL, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + }, + }, + }, +} + +var CreateTableOp30 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "a", + Type: "int", + Nullable: true, + }, + { + Name: "b", + Type: "int", + Nullable: true, + }, + { + Name: "c", + Type: "int", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeForeignKey, + Columns: []string{"a", "b", "c"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + References: &migrations.ConstraintReferences{ + Table: "bar", + Columns: []string{"b", "c", "d"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + }, + }, + }, +} + +var CreateTableOp31 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "a", + Type: "int", + Nullable: true, + }, + { + Name: "b", + Type: "int", + Nullable: true, + }, + { + Name: "c", + Type: "int", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeForeignKey, + Columns: []string{"a", "b", "c"}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + References: &migrations.ConstraintReferences{ + Table: "bar", + Columns: []string{"b", "c", "d"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + MatchType: migrations.ConstraintReferencesMatchTypeFULL, + }, + }, + }, +} diff --git a/schema.json b/schema.json index 4ea9d7ea7..d63099190 100644 --- a/schema.json +++ b/schema.json @@ -167,7 +167,7 @@ "default": "SIMPLE" }, "on_update": { - "description": "On delete behavior of the foreign key constraint", + "description": "On update behavior of the foreign key constraint", "$ref": "#/$defs/ForeignKeyReferenceOnDelete", "default": "NO ACTION" }, @@ -175,6 +175,13 @@ "description": "On delete behavior of the foreign key constraint", "$ref": "#/$defs/ForeignKeyReferenceOnDelete", "default": "NO ACTION" + }, + "on_delete_set_columns": { + "description": "Columns to set to null or to default on delete", + "type": "array", + "items": { + "type": "string" + } } } }, From 1a9eb26ce111c327afab73eae4fb94a88d7bb1d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 18:43:09 +0100 Subject: [PATCH 39/66] Document new option on_delete_set_columns --- docs/operations/create_table.mdx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index 2a8a8d7a1..c49b03fef 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -34,7 +34,7 @@ Each `column` is defined as: "name": "name of foreign key constraint", "table": "name of referenced table", "column": "name of referenced column", - "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", + "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, SET DEFAULT, RESTRICT, or NO ACTION. Default is NO ACTION", } }, ``` @@ -57,8 +57,9 @@ Each `constraint` is defined as: "name": "name of foreign key constraint", "table": "name of referenced table", "columns": ["list", "of", "referenced", "columns"], - "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", - "on_update": "ON UPDATE behaviour, can be CASCADE, SET NULL, RESTRICT, or NO ACTION. Default is NO ACTION", + "on_delete": "ON DELETE behaviour, can be CASCADE, SET NULL, SET DEFAULT, RESTRICT, or NO ACTION. Default is NO ACTION", + "on_delete_set_columns": ["list of FKs to set", "in on delete operation on SET NULL or SET DEFAULT"], + "on_update": "ON UPDATE behaviour, can be CASCADE, SET NULL, SET DEFAULT, RESTRICT, or NO ACTION. Default is NO ACTION", "match_type": "match type, can be SIMPLE, FULL or PARTIAL. Default is SIMPLE" }, "index_parameters": { @@ -143,4 +144,4 @@ Create a table with table level constraints: Create a table with table level foreign key constraints: - \ No newline at end of file + From 0ee70ea08969a65856a324dbf036430d3c406cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 19:05:50 +0100 Subject: [PATCH 40/66] Add more assertions to e2e test --- pkg/migrations/op_common_test.go | 39 ++++++++++++++++++++++++++ pkg/migrations/op_create_table_test.go | 24 ++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 34788a9bf..89dad0b56 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -250,6 +250,13 @@ func NotValidatedForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, co } } +func TableForeignKeyMustExist(t *testing.T, db *sql.DB, schema, table, constraint string, deferrable, initiallyDeferred bool) { + t.Helper() + if !tableForeignKeyExists(t, db, schema, table, constraint, deferrable, initiallyDeferred) { + t.Fatalf("Expected table foreign key %q to exist", constraint) + } +} + func PrimaryKeyConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) { t.Helper() if !primaryKeyConstraintExists(t, db, schema, table, constraint) { @@ -410,6 +417,38 @@ func foreignKeyExists(t *testing.T, db *sql.DB, schema, table, constraint string return exists } +func tableForeignKeyExists(t *testing.T, db *sql.DB, schema, table, constraint string, deferrable, initiallyDeferred bool) bool { + t.Helper() + + deferrableStr := "NO" + if deferrable { + deferrableStr = "YES" + } + initiallyDeferredStr := "NO" + if initiallyDeferred { + initiallyDeferredStr = "YES" + } + + var exists bool + err := db.QueryRow(` + SELECT EXISTS ( + SELECT 1 + FROM information_schema.table_constraints + WHERE table_schema = $1 + AND table_name = $2 + AND constraint_name = $3 + AND constraint_type = 'FOREIGN KEY' + AND is_deferrable = $4 + AND initially_deferred = $5 + )`, + schema, table, constraint, deferrableStr, initiallyDeferredStr).Scan(&exists) + if err != nil { + t.Fatal(err) + } + + return exists +} + func primaryKeyConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint string) bool { t.Helper() diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 29a034c24..b9c8cd43a 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -829,24 +829,44 @@ func TestCreateTable(t *testing.T) { }, }, afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Table level FK exists on the new table. + TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", false, false) + MustInsert(t, db, schema, "01_create_referenced_table", "owners", map[string]string{ "id": "1", "name": "alice", "city": "new york", }) + + // Inserting a row into the referencing table succeeds as the referenced row exists. + MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "1", + "owner_id": "1", + "name": "good boy", + }) + + // Inserting a row into the referencing table fails as the referenced row does not exist. + MustNotInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "1", + "owner_id": "0", + "name": "spider pig", + }, testutils.FKViolationErrorCode) }, afterRollback: func(t *testing.T, db *sql.DB, schema string) { // The table has been dropped, so the FK constraint is gone. }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { - // Inserting a row into the table succeeds when the PK constraint is satisfied. + // Table level FK exists on the new table. + TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", false, false) + + // Inserting a row into the table succeeds when the FK constraint is satisfied. MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ "id": "1", "owner_id": "1", "name": "cutie pie", }) - // Inserting a row into the table fails when the PK constraint is not satisfied. + // Inserting a row into the table fails when the FK constraint is not satisfied. MustNotInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ "id": "2", "owner_id": "0", From b47ade4b1a0bd2e7b9eda412bc161603138c9a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 19:18:46 +0100 Subject: [PATCH 41/66] fix --- pkg/sql2pgroll/expect/create_table.go | 44 +++++++++++++++------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index d88877ba1..f48c35358 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -449,11 +449,12 @@ var CreateTableOp27 = &migrations.OpCreateTable{ InitiallyDeferred: false, NoInherit: false, References: &migrations.ConstraintReferences{ - Table: "bar", - Columns: []string{"b"}, - OnDelete: migrations.ForeignKeyReferenceOnDeleteNOACTION, - OnUpdate: migrations.ForeignKeyReferenceOnDeleteNOACTION, - MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + Table: "bar", + Columns: []string{"b"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteNOACTION, + OnDeleteSetColumns: []string{}, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteNOACTION, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, }, }, }, @@ -477,11 +478,12 @@ var CreateTableOp28 = &migrations.OpCreateTable{ InitiallyDeferred: false, NoInherit: false, References: &migrations.ConstraintReferences{ - Table: "bar", - Columns: []string{"b"}, - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, - OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, - MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + Table: "bar", + Columns: []string{"b"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnDeleteSetColumns: []string{}, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, }, }, }, @@ -554,11 +556,12 @@ var CreateTableOp30 = &migrations.OpCreateTable{ InitiallyDeferred: false, NoInherit: false, References: &migrations.ConstraintReferences{ - Table: "bar", - Columns: []string{"b", "c", "d"}, - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, - OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, - MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, + Table: "bar", + Columns: []string{"b", "c", "d"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnDeleteSetColumns: []string{}, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, }, }, }, @@ -592,11 +595,12 @@ var CreateTableOp31 = &migrations.OpCreateTable{ InitiallyDeferred: false, NoInherit: false, References: &migrations.ConstraintReferences{ - Table: "bar", - Columns: []string{"b", "c", "d"}, - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, - OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, - MatchType: migrations.ConstraintReferencesMatchTypeFULL, + Table: "bar", + Columns: []string{"b", "c", "d"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnDeleteSetColumns: []string{}, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + MatchType: migrations.ConstraintReferencesMatchTypeFULL, }, }, }, From eb3965d96d0ca3c033c43a8575290caff15e0840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 19:26:19 +0100 Subject: [PATCH 42/66] adjust expected --- pkg/sql2pgroll/expect/create_table.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index f48c35358..db823a8c5 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -518,7 +518,7 @@ var CreateTableOp29 = &migrations.OpCreateTable{ NoInherit: false, References: &migrations.ConstraintReferences{ Table: "bar", - Columns: []string{"b", "c", "d"}, + Columns: []string{"a", "b"}, OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, OnDeleteSetColumns: []string{"b"}, OnUpdate: migrations.ForeignKeyReferenceOnDeleteSETNULL, @@ -557,7 +557,7 @@ var CreateTableOp30 = &migrations.OpCreateTable{ NoInherit: false, References: &migrations.ConstraintReferences{ Table: "bar", - Columns: []string{"b", "c", "d"}, + Columns: []string{"d", "e", "f"}, OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, OnDeleteSetColumns: []string{}, OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, @@ -596,10 +596,10 @@ var CreateTableOp31 = &migrations.OpCreateTable{ NoInherit: false, References: &migrations.ConstraintReferences{ Table: "bar", - Columns: []string{"b", "c", "d"}, - OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, + Columns: []string{"d", "e", "f"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteNOACTION, OnDeleteSetColumns: []string{}, - OnUpdate: migrations.ForeignKeyReferenceOnDeleteCASCADE, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteNOACTION, MatchType: migrations.ConstraintReferencesMatchTypeFULL, }, }, From 115af43975525f919839b58492fc421328e74eb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 19:31:02 +0100 Subject: [PATCH 43/66] mr --- pkg/sql2pgroll/expect/create_table.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index db823a8c5..324768649 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -502,26 +502,21 @@ var CreateTableOp29 = &migrations.OpCreateTable{ Type: "int", Nullable: true, }, - { - Name: "c", - Type: "int", - Nullable: true, - }, }, Constraints: []migrations.Constraint{ { Type: migrations.ConstraintTypeForeignKey, - Columns: []string{"a", "b", "c"}, + Columns: []string{"a", "b"}, NullsNotDistinct: false, Deferrable: false, InitiallyDeferred: false, NoInherit: false, References: &migrations.ConstraintReferences{ Table: "bar", - Columns: []string{"a", "b"}, + Columns: []string{"c", "d"}, OnDelete: migrations.ForeignKeyReferenceOnDeleteSETNULL, OnDeleteSetColumns: []string{"b"}, - OnUpdate: migrations.ForeignKeyReferenceOnDeleteSETNULL, + OnUpdate: migrations.ForeignKeyReferenceOnDeleteNOACTION, MatchType: migrations.ConstraintReferencesMatchTypeSIMPLE, }, }, From ed04cdf7a335b38150fcbd1ac5e116d24aa46d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 19:41:33 +0100 Subject: [PATCH 44/66] improve validation --- pkg/migrations/op_create_table.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index b666f59a6..a6e163282 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -168,6 +168,14 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { if c.References == nil { return FieldRequiredError{Name: "references"} } + if len(c.References.OnDeleteSetColumns) != 0 { + if c.References.OnDelete != ForeignKeyReferenceOnDeleteSETDEFAULT && c.References.OnDelete != ForeignKeyReferenceOnDeleteSETNULL { + return InvalidOnDeleteSetColumnError{ + Name: o.Name, + } + } + } + } } From 1da339aa0d94142944d95cbbe16e547470427831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Tue, 21 Jan 2025 19:46:26 +0100 Subject: [PATCH 45/66] add missing file --- pkg/migrations/errors.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index f73a829ee..81806ff5e 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -208,6 +208,18 @@ func (e InvalidOnDeleteSettingError) Error() string { ) } +type InvalidOnDeleteSetColumnError struct { + Name string +} + +func (e InvalidOnDeleteSetColumnError) Error() string { + return fmt.Sprintf("if on_delete_set_columns is set in foreign key %q, on_delete setting must be one of: %q, %q", + e.Name, + ForeignKeyReferenceOnDeleteSETDEFAULT, + ForeignKeyReferenceOnDeleteSETNULL, + ) +} + type AlterColumnNoChangesError struct { Table string Column string From 2a76649a52c3598e935b7b992b7fd32e5fc69f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 22 Jan 2025 14:32:43 +0100 Subject: [PATCH 46/66] support setting columns --- pkg/migrations/errors.go | 16 ++- pkg/migrations/op_create_table.go | 18 +++- pkg/migrations/op_create_table_test.go | 137 +++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 5 deletions(-) diff --git a/pkg/migrations/errors.go b/pkg/migrations/errors.go index 81806ff5e..e7785f334 100644 --- a/pkg/migrations/errors.go +++ b/pkg/migrations/errors.go @@ -208,11 +208,11 @@ func (e InvalidOnDeleteSettingError) Error() string { ) } -type InvalidOnDeleteSetColumnError struct { +type UnexpectedOnDeleteSetColumnError struct { Name string } -func (e InvalidOnDeleteSetColumnError) Error() string { +func (e UnexpectedOnDeleteSetColumnError) Error() string { return fmt.Sprintf("if on_delete_set_columns is set in foreign key %q, on_delete setting must be one of: %q, %q", e.Name, ForeignKeyReferenceOnDeleteSETDEFAULT, @@ -220,6 +220,18 @@ func (e InvalidOnDeleteSetColumnError) Error() string { ) } +type InvalidOnDeleteSetColumnError struct { + Name string + Column string +} + +func (e InvalidOnDeleteSetColumnError) Error() string { + return fmt.Sprintf("invalid on_delete_set_columns setting: column %q is not part of the foreign key %q", + e.Column, + e.Name, + ) +} + type AlterColumnNoChangesError struct { Table string Column string diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index a6e163282..de7de593c 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -5,6 +5,7 @@ package migrations import ( "context" "fmt" + "slices" "strings" "github.com/lib/pq" @@ -170,10 +171,18 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error { } if len(c.References.OnDeleteSetColumns) != 0 { if c.References.OnDelete != ForeignKeyReferenceOnDeleteSETDEFAULT && c.References.OnDelete != ForeignKeyReferenceOnDeleteSETNULL { - return InvalidOnDeleteSetColumnError{ + return UnexpectedOnDeleteSetColumnError{ Name: o.Name, } } + for _, col := range c.References.OnDeleteSetColumns { + if !slices.Contains(c.Columns, col) { + return InvalidOnDeleteSetColumnError{ + Name: o.Name, + Column: col, + } + } + } } } @@ -296,7 +305,7 @@ func constraintsToSQL(constraints []Constraint) (string, error) { case ConstraintTypePrimaryKey: constraintsSQL[i] = writer.WritePrimaryKey() case ConstraintTypeForeignKey: - constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete, c.References.OnUpdate) + constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete, c.References.OnUpdate, c.References.OnDeleteSetColumns) } } if len(constraintsSQL) == 0 { @@ -355,10 +364,13 @@ func (w *ConstraintSQLWriter) WritePrimaryKey() string { return constraint } -func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDelete, onUpdate ForeignKeyReferenceOnDelete) string { +func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referencedColumns []string, onDelete, onUpdate ForeignKeyReferenceOnDelete, setColumns []string) string { onDeleteAction := string(ForeignKeyReferenceOnDeleteNOACTION) if onDelete != "" { onDeleteAction = strings.ToUpper(string(onDelete)) + if len(setColumns) != 0 { + onDeleteAction += " (" + strings.Join(quoteColumnNames(setColumns), ", ") + ")" + } } onUpdateAction := string(ForeignKeyReferenceOnDeleteNOACTION) if onUpdate != "" { diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index b9c8cd43a..fb43d4f30 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -882,6 +882,143 @@ func TestCreateTable(t *testing.T) { assert.Equal(t, []map[string]any{}, rows) }, }, + { + name: "create table with multi-column foreign key constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_referenced_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "owners", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + }, + { + Name: "name", + Type: "text", + }, + { + Name: "city", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "pk_owners", + Type: migrations.ConstraintTypePrimaryKey, + Columns: []string{"id", "city"}, + }, + }, + }, + }, + }, + { + Name: "02_create_referencing_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "pets", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + Pk: true, + }, + { + Name: "owner_id", + Nullable: true, + Type: "int", + }, + { + Name: "owner_city_id", + Type: "text", + Default: ptr("'chicago'"), + }, + { + Name: "name", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "fk_owners", + Type: migrations.ConstraintTypeForeignKey, + Columns: []string{"owner_id", "owner_city_id"}, + Deferrable: true, + References: &migrations.ConstraintReferences{ + Table: "owners", + Columns: []string{"id", "city"}, + OnDelete: migrations.ForeignKeyReferenceOnDeleteSETDEFAULT, + OnDeleteSetColumns: []string{"owner_id", "owner_city_id"}, + }, + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // Table level FK exists on the new table. + TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", true, false) + + MustInsert(t, db, schema, "01_create_referenced_table", "owners", map[string]string{ + "id": "1", + "name": "alice", + "city": "new york", + }) + + // Inserting a row into the referencing table succeeds as the referenced row exists. + MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "1", + "owner_id": "1", + "owner_city_id": "new york", + "name": "good boy", + }) + + // Inserting a row into the referencing table fails as the referenced row does not exist. + MustNotInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "2", + "owner_id": "0", + "owner_city_id": "sacramento", + "name": "spider pig", + }, testutils.FKViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the FK constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // Table level FK exists on the new table. + TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", true, false) + + // Inserting a row into the table succeeds when the FK constraint is satisfied. + MustInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "1", + "owner_id": "1", + "owner_city_id": "new york", + "name": "cutie pie", + }) + + // Inserting a row into the table fails when the FK constraint is not satisfied. + MustNotInsert(t, db, schema, "02_create_referencing_table", "pets", map[string]string{ + "id": "2", + "owner_id": "0", + "owner_city_id": "sacramento", + "name": "bobby", + }, testutils.FKViolationErrorCode) + + // Deleting the row from the referenced table cascades to the referencing table. + MustDelete(t, db, schema, "02_create_referencing_table", "owners", map[string]string{ + "id": "1", + }) + + // deleting owner does not cascade to pets and default values are set. + rows := MustSelect(t, db, schema, "02_create_referencing_table", "pets") + assert.Equal(t, []map[string]any{ + {"id": 1, "owner_id": nil, "owner_city_id": "chicago", "name": "cutie pie"}, + }, rows) + }, + }, }) } From 6e03cb5b7df43385709d223c7f72331a4191e137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 22 Jan 2025 15:43:53 +0100 Subject: [PATCH 47/66] skip new tests --- pkg/migrations/op_create_table_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index fb43d4f30..8871fa156 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -4,6 +4,7 @@ package migrations_test import ( "database/sql" + "os" "strings" "testing" @@ -959,6 +960,10 @@ func TestCreateTable(t *testing.T) { }, }, afterStart: func(t *testing.T, db *sql.DB, schema string) { + if strings.HasPrefix(os.Getenv("POSTGRES_VERSION"), "14.") { + t.Skip("ON DELETE SET DEFAULT is not supported in PostgreSQL 14") + } + // Table level FK exists on the new table. TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", true, false) @@ -988,6 +993,10 @@ func TestCreateTable(t *testing.T) { // The table has been dropped, so the FK constraint is gone. }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { + if strings.HasPrefix(os.Getenv("POSTGRES_VERSION"), "14.") { + t.Skip("ON DELETE SET DEFAULT is not supported in PostgreSQL 14") + } + // Table level FK exists on the new table. TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", true, false) From b30a1b5ad0e084df45a023d2ccd1c606bf21c38a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 22 Jan 2025 15:50:52 +0100 Subject: [PATCH 48/66] mivan? --- pkg/migrations/op_create_table_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 8871fa156..bca714842 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -4,6 +4,7 @@ package migrations_test import ( "database/sql" + "fmt" "os" "strings" "testing" @@ -960,6 +961,7 @@ func TestCreateTable(t *testing.T) { }, }, afterStart: func(t *testing.T, db *sql.DB, schema string) { + fmt.Println(os.Getenv("POSTGRES_VERSION")) if strings.HasPrefix(os.Getenv("POSTGRES_VERSION"), "14.") { t.Skip("ON DELETE SET DEFAULT is not supported in PostgreSQL 14") } From 3b0d39e08968f44d7d3e854ba7f8d906ed4bd61f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 22 Jan 2025 16:01:13 +0100 Subject: [PATCH 49/66] put support in the correct place --- pkg/migrations/op_common_test.go | 28 ++++++++++++++++++-------- pkg/migrations/op_create_table_test.go | 14 ++----------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 89dad0b56..5c4276c42 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -8,7 +8,10 @@ import ( "errors" "fmt" "maps" + "os" "slices" + "strconv" + "strings" "testing" "github.com/lib/pq" @@ -20,14 +23,15 @@ import ( ) type TestCase struct { - name string - migrations []migrations.Migration - wantStartErr error - wantRollbackErr error - wantCompleteErr error - afterStart func(t *testing.T, db *sql.DB, schema string) - afterComplete func(t *testing.T, db *sql.DB, schema string) - afterRollback func(t *testing.T, db *sql.DB, schema string) + name string + minPgMajorVersion int + migrations []migrations.Migration + wantStartErr error + wantRollbackErr error + wantCompleteErr error + afterStart func(t *testing.T, db *sql.DB, schema string) + afterComplete func(t *testing.T, db *sql.DB, schema string) + afterRollback func(t *testing.T, db *sql.DB, schema string) } type TestCases []TestCase @@ -40,6 +44,14 @@ func ExecuteTests(t *testing.T, tests TestCases, opts ...roll.Option) { testSchema := testutils.TestSchema() for _, tt := range tests { + if tt.minPgMajorVersion != 0 && os.Getenv("POSTGRES_VERSION") != "" { + pgMajorVersion := strings.Split(os.Getenv("POSTGRES_VERSION"), ".")[0] + version, _ := strconv.Atoi(pgMajorVersion) + if version < tt.minPgMajorVersion { + t.Skipf("Skipping test %q for PostgreSQL version %s", tt.name, os.Getenv("POSTGRES_VERSION")) + } + } + t.Run(tt.name, func(t *testing.T) { testutils.WithMigratorInSchemaAndConnectionToContainerWithOptions(t, testSchema, opts, func(mig *roll.Roll, db *sql.DB) { ctx := context.Background() diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index bca714842..b2081a306 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -4,8 +4,6 @@ package migrations_test import ( "database/sql" - "fmt" - "os" "strings" "testing" @@ -885,7 +883,8 @@ func TestCreateTable(t *testing.T) { }, }, { - name: "create table with multi-column foreign key constraint", + name: "create table with multi-column foreign key constraint", + minPgMajorVersion: 15, migrations: []migrations.Migration{ { Name: "01_create_referenced_table", @@ -961,11 +960,6 @@ func TestCreateTable(t *testing.T) { }, }, afterStart: func(t *testing.T, db *sql.DB, schema string) { - fmt.Println(os.Getenv("POSTGRES_VERSION")) - if strings.HasPrefix(os.Getenv("POSTGRES_VERSION"), "14.") { - t.Skip("ON DELETE SET DEFAULT is not supported in PostgreSQL 14") - } - // Table level FK exists on the new table. TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", true, false) @@ -995,10 +989,6 @@ func TestCreateTable(t *testing.T) { // The table has been dropped, so the FK constraint is gone. }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { - if strings.HasPrefix(os.Getenv("POSTGRES_VERSION"), "14.") { - t.Skip("ON DELETE SET DEFAULT is not supported in PostgreSQL 14") - } - // Table level FK exists on the new table. TableForeignKeyMustExist(t, db, schema, "pets", "fk_owners", true, false) From 312cf21a511bd05537f37079017dff9c88336a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Wed, 22 Jan 2025 17:14:35 +0100 Subject: [PATCH 50/66] init --- pkg/migrations/op_create_table.go | 37 ++++++++++++++++++++++++++----- pkg/migrations/types.go | 15 +++++++++++++ pkg/schema/schema.go | 18 +++++++++++++++ schema.json | 23 ++++++++++++++++++- 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index de7de593c..078bb8857 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -215,6 +215,7 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { uniqueConstraints := make(map[string]*schema.UniqueConstraint, 0) checkConstraints := make(map[string]*schema.CheckConstraint, 0) foreignKeys := make(map[string]*schema.ForeignKey, 0) + excludeConstraints := make(map[string]*schema.ExcludeConstraint, 0) for _, c := range o.Constraints { switch c.Type { case ConstraintTypeUnique: @@ -240,16 +241,24 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema { OnUpdate: string(c.References.OnUpdate), MatchType: string(c.References.MatchType), } + case ConstraintTypeExclude: + excludeConstraints[c.Name] = &schema.ExcludeConstraint{ + Name: c.Name, + IndexMethod: c.Exclude.IndexMethod, + Elements: c.Exclude.Elements, + Predicate: c.Exclude.Predicate, + } } } s.AddTable(o.Name, &schema.Table{ - Name: o.Name, - Columns: columns, - UniqueConstraints: uniqueConstraints, - CheckConstraints: checkConstraints, - PrimaryKey: primaryKeys, - ForeignKeys: foreignKeys, + Name: o.Name, + Columns: columns, + UniqueConstraints: uniqueConstraints, + CheckConstraints: checkConstraints, + PrimaryKey: primaryKeys, + ForeignKeys: foreignKeys, + ExcludeConstraints: excludeConstraints, }) return s @@ -306,6 +315,8 @@ func constraintsToSQL(constraints []Constraint) (string, error) { constraintsSQL[i] = writer.WritePrimaryKey() case ConstraintTypeForeignKey: constraintsSQL[i] = writer.WriteForeignKey(c.References.Table, c.References.Columns, c.References.OnDelete, c.References.OnUpdate, c.References.OnDeleteSetColumns) + case ConstraintTypeExclude: + constraintsSQL[i] = writer.WriteExclude(c.Exclude.IndexMethod, c.Exclude.Elements, c.Exclude.Predicate) } } if len(constraintsSQL) == 0 { @@ -392,6 +403,20 @@ func (w *ConstraintSQLWriter) WriteForeignKey(referencedTable string, referenced return constraint } +func (w *ConstraintSQLWriter) WriteExclude(indexMethod, elements, predicate string) string { + constraint := "" + if w.Name != "" { + constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) + } + constraint += fmt.Sprintf("EXCLUDE USING %s (%s)", pq.QuoteIdentifier(w.Name), indexMethod, elements) + constraint += w.addIndexParameters() + if predicate != "" { + constraint += fmt.Sprintf(" WHERE (%s)", predicate) + } + constraint += w.addDeferrable() + return constraint +} + func (w *ConstraintSQLWriter) addIndexParameters() string { constraint := "" if len(w.IncludeColumns) != 0 { diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index a89f4cefe..434122507 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -55,6 +55,9 @@ type Constraint struct { // Deferable constraint Deferrable bool `json:"deferrable,omitempty"` + // Exclude corresponds to the JSON schema field "exclude". + Exclude *ConstraintExclude `json:"exclude,omitempty"` + // IndexParameters corresponds to the JSON schema field "index_parameters". IndexParameters *ConstraintIndexParameters `json:"index_parameters,omitempty"` @@ -77,6 +80,17 @@ type Constraint struct { Type ConstraintType `json:"type"` } +type ConstraintExclude struct { + // Columns of the exclude constraint + Elements string `json:"elements,omitempty"` + + // Index method + IndexMethod string `json:"index_method,omitempty"` + + // Predicate for the exclusion constraint + Predicate string `json:"predicate,omitempty"` +} + type ConstraintIndexParameters struct { // IncludeColumns corresponds to the JSON schema field "include_columns". IncludeColumns []string `json:"include_columns,omitempty"` @@ -118,6 +132,7 @@ const ConstraintReferencesMatchTypeSIMPLE ConstraintReferencesMatchType = "SIMPL type ConstraintType string const ConstraintTypeCheck ConstraintType = "check" +const ConstraintTypeExclude ConstraintType = "exclude" const ConstraintTypeForeignKey ConstraintType = "foreign_key" const ConstraintTypePrimaryKey ConstraintType = "primary_key" const ConstraintTypeUnique ConstraintType = "unique" diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index e9a1daae1..4b7bce6fe 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -59,6 +59,9 @@ type Table struct { // UniqueConstraints is a map of all unique constraints defined on the table UniqueConstraints map[string]*UniqueConstraint `json:"uniqueConstraints"` + // ExcludeConstraints is a map of all exclude constraints defined on the table + ExcludeConstraints map[string]*ExcludeConstraint `json:"excludeConstraints"` + // Whether or not the table has been deleted in the virtual schema Deleted bool `json:"-"` } @@ -154,6 +157,21 @@ type UniqueConstraint struct { Columns []string `json:"columns"` } +// UniqueConstraint represents a unique constraint on a table +type ExcludeConstraint struct { + // Name is the name of the exclude constraint in postgres + Name string `json:"name"` + + // IndexMethod is the index method of the exclude constraint + IndexMethod string `json:"indexMethod"` + + // Elements is the elements of the exclude constraint + Elements string `json:"elements"` + + // Predicate is the predicate of the index + Predicate string `json:"predicate"` +} + // GetTable returns a table by name func (s *Schema) GetTable(name string) *Table { if s.Tables == nil { diff --git a/schema.json b/schema.json index d63099190..0b2a9c77e 100644 --- a/schema.json +++ b/schema.json @@ -116,7 +116,7 @@ "type": { "description": "Type of the constraint", "type": "string", - "enum": ["unique", "check", "primary_key", "foreign_key"] + "enum": ["unique", "check", "primary_key", "foreign_key", "exclude"] }, "deferrable": { "description": "Deferable constraint", @@ -185,6 +185,27 @@ } } }, + "exclude": { + "type": "object", + "additionalProperties": false, + "properties": { + "index_method": { + "description": "Index method", + "type": "string", + "default": "" + }, + "elements": { + "type": "string", + "default": "", + "description": "Columns of the exclude constraint" + }, + "predicate": { + "type": "string", + "description": "Predicate for the exclusion constraint", + "default": "" + } + } + }, "index_parameters": { "type": "object", "additionalProperties": false, From 51ce6a5c174f7387cda1b19541a8966d07eee7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 11:42:11 +0100 Subject: [PATCH 51/66] add exclude constraint --- go.mod | 2 +- go.sum | 2 + internal/testutils/error_codes.go | 13 +- internal/testutils/util.go | 6 + pkg/migrations/op_common_test.go | 27 ++++ pkg/migrations/op_create_table.go | 4 +- pkg/migrations/op_create_table_test.go | 184 +++++++++++++++++++++++++ pkg/sql2pgroll/create_table.go | 31 +++++ pkg/sql2pgroll/create_table_test.go | 8 ++ pkg/sql2pgroll/expect/create_table.go | 72 ++++++++++ schema.json | 26 ++++ 11 files changed, 367 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index c3fe49305..2673f0808 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/stretchr/testify v1.10.0 github.com/testcontainers/testcontainers-go v0.35.0 github.com/testcontainers/testcontainers-go/modules/postgres v0.35.0 - github.com/xataio/pg_query_go/v6 v6.0.0-20250122133641-54118c062181 + github.com/xataio/pg_query_go/v6 v6.0.0-20250123182324-526a22cbe0c0 golang.org/x/tools v0.29.0 ) diff --git a/go.sum b/go.sum index 88fb36390..8b390327f 100644 --- a/go.sum +++ b/go.sum @@ -223,6 +223,8 @@ github.com/tklauser/numcpus v0.8.0 h1:Mx4Wwe/FjZLeQsK/6kt2EOepwwSl7SmJrK5bV/dXYg github.com/tklauser/numcpus v0.8.0/go.mod h1:ZJZlAY+dmR4eut8epnzf0u/VwodKmryxR8txiloSqBE= github.com/xataio/pg_query_go/v6 v6.0.0-20250122133641-54118c062181 h1:iLOHgul20WFUhO4eFpJ/lZRkHzZICF2ghzncxtOcD0E= github.com/xataio/pg_query_go/v6 v6.0.0-20250122133641-54118c062181/go.mod h1:GK6bpfAhPtZb7wG/IccqvnH+cz3cmvvRTkC+MosESGo= +github.com/xataio/pg_query_go/v6 v6.0.0-20250123182324-526a22cbe0c0 h1:/fXLU7NusFv8TSEAM7n+vtsN5Rr2La9tXWEunrJOWrI= +github.com/xataio/pg_query_go/v6 v6.0.0-20250123182324-526a22cbe0c0/go.mod h1:GK6bpfAhPtZb7wG/IccqvnH+cz3cmvvRTkC+MosESGo= github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778/go.mod h1:2MuV+tbUrU1zIOPMxZ5EncGwgmMJsa+9ucAQZXxsObs= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= diff --git a/internal/testutils/error_codes.go b/internal/testutils/error_codes.go index cd6b54ea6..d2465f87a 100644 --- a/internal/testutils/error_codes.go +++ b/internal/testutils/error_codes.go @@ -3,10 +3,11 @@ package testutils const ( - CheckViolationErrorCode string = "check_violation" - FKViolationErrorCode string = "foreign_key_violation" - NotNullViolationErrorCode string = "not_null_violation" - UniqueViolationErrorCode string = "unique_violation" - UndefinedColumnErrorCode string = "undefined_column" - UndefinedTableErrorCode string = "undefined_table" + CheckViolationErrorCode string = "check_violation" + ExclusionViolationErrorCode string = "exclusion_violation" + FKViolationErrorCode string = "foreign_key_violation" + NotNullViolationErrorCode string = "not_null_violation" + UndefinedColumnErrorCode string = "undefined_column" + UndefinedTableErrorCode string = "undefined_table" + UniqueViolationErrorCode string = "unique_violation" ) diff --git a/internal/testutils/util.go b/internal/testutils/util.go index 4893878a4..5dc45b247 100644 --- a/internal/testutils/util.go +++ b/internal/testutils/util.go @@ -187,6 +187,12 @@ func WithMigratorInSchemaAndConnectionToContainerWithOptions(t testing.TB, schem t.Fatal(err) } + // add extension for btree_gist + _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION btree_gist SCHEMA %s", schema)) + if err != nil { + t.Fatal(err) + } + fn(mig, db) } diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index fd7712249..6f81f7bd2 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -272,6 +272,13 @@ func PrimaryKeyConstraintMustExist(t *testing.T, db *sql.DB, schema, table, cons } } +func ExcludeConstraintMustExist(t *testing.T, db *sql.DB, schema, table, constraint string) { + t.Helper() + if !excludeConstraintExists(t, db, schema, table, constraint) { + t.Fatalf("Expected constraint %q to exist", constraint) + } +} + func IndexMustExist(t *testing.T, db *sql.DB, schema, table, index string) { t.Helper() if !indexExists(t, db, schema, table, index) { @@ -477,6 +484,26 @@ func primaryKeyConstraintExists(t *testing.T, db *sql.DB, schema, table, constra return exists } +func excludeConstraintExists(t *testing.T, db *sql.DB, schema, table, constraint string) bool { + t.Helper() + + var exists bool + err := db.QueryRow(` + SELECT EXISTS ( + SELECT 1 + FROM pg_catalog.pg_constraint + WHERE conrelid = $1::regclass + AND conname = $2 + AND contype = 'x' + )`, + fmt.Sprintf("%s.%s", schema, table), constraint).Scan(&exists) + if err != nil { + t.Fatal(err) + } + + return exists +} + func triggerExists(t *testing.T, db *sql.DB, schema, table, trigger string) bool { t.Helper() diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 77ed3a022..a780ac596 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -27,6 +27,8 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri if err != nil { return nil, fmt.Errorf("failed to create constraints SQL: %w", err) } + fmt.Println(columnsSQL) + fmt.Println(constraintsSQL) // Create the table _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s %s)", @@ -413,7 +415,7 @@ func (w *ConstraintSQLWriter) WriteExclude(indexMethod, elements, predicate stri if w.Name != "" { constraint = fmt.Sprintf("CONSTRAINT %s ", pq.QuoteIdentifier(w.Name)) } - constraint += fmt.Sprintf("EXCLUDE USING %s (%s)", pq.QuoteIdentifier(w.Name), indexMethod, elements) + constraint += fmt.Sprintf("EXCLUDE USING %s (%s)", indexMethod, elements) constraint += w.addIndexParameters() if predicate != "" { constraint += fmt.Sprintf(" WHERE (%s)", predicate) diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index b2081a306..a377dee7b 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -1020,6 +1020,190 @@ func TestCreateTable(t *testing.T) { }, rows) }, }, + { + name: "create table with a simple exclude constraint mimicking unique constraint", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + }, + { + Name: "name", + Type: "text", + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "exclude_id", + Type: migrations.ConstraintTypeExclude, + Exclude: &migrations.ConstraintExclude{ + IndexMethod: "btree", + Elements: "id WITH =", + }, + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The exclusion constraint exists on the new table. + ExcludeConstraintMustExist(t, db, schema, "users", "exclude_id") + + // Inserting a row into the table succeeds when the exclude constraint is satisfied. + // This is the first row, so the constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "1", + "name": "alice", + }) + + // Inserting a row into the table fails because there is already a row with id 1. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "1", + "name": "b", + }, testutils.ExclusionViolationErrorCode) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The exclusion constraint exists on the new table. + ExcludeConstraintMustExist(t, db, schema, "users", "exclude_id") + + // Inserting a row into the table succeeds when the exclude constraint is satisfied. + // This is the first row, so the constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "1", + "name": "alice", + }) + + // Inserting a row into the table fails because there is already a row with id 1. + MustNotInsert(t, db, schema, "01_create_table", "users", map[string]string{ + "id": "1", + "name": "b", + }, testutils.ExclusionViolationErrorCode) + }, + }, + { + name: "create table with an exclude constraint to check overlap", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "reservations", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + }, + { + Name: "start", + Type: "timestamp", + }, + { + Name: "finish", + Type: "timestamp", + }, + { + Name: "canceled", + Type: "bool", + Default: ptr("false"), + }, + }, + Constraints: []migrations.Constraint{ + { + Name: "forbid_reservation_overlap", + Type: migrations.ConstraintTypeExclude, + Exclude: &migrations.ConstraintExclude{ + IndexMethod: "gist", + Elements: "id WITH =, tsrange(start, finish) WITH &&", + Predicate: "NOT canceled", + }, + }, + }, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The exclusion constraint exists on the new table. + ExcludeConstraintMustExist(t, db, schema, "reservations", "forbid_reservation_overlap") + + // Inserting a row into the table succeeds when the exclude constraint is satisfied. + // This is the first row, so the constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "1", + "start": "2020-01-01T00:00:00Z", + "finish": "2020-01-02T00:00:00Z", + }) + + // Inserting a row into the table fails because there is a reservation that overlaps with the new one. + MustNotInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "1", + "start": "2020-01-01T08:00:00Z", + "finish": "2020-01-02T08:00:00Z", + }, testutils.ExclusionViolationErrorCode) + + // Insert a row that overlaps with the existing one, but is canceled. + MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "1", + "start": "2020-01-01T08:00:00Z", + "finish": "2020-01-02T08:00:00Z", + "canceled": "true", + }) + + // Insert a row that overlaps with the existing one, but with a different id. + MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "2", + "start": "2020-01-01T08:00:00Z", + "finish": "2020-01-02T08:00:00Z", + }) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The table has been dropped, so the constraint is gone. + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The exclusion constraint exists on the new table. + ExcludeConstraintMustExist(t, db, schema, "reservations", "forbid_reservation_overlap") + + // Inserting a row into the table succeeds when the exclude constraint is satisfied. + // This is the first row, so the constraint is satisfied. + MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "1", + "start": "2020-01-01T00:00:00Z", + "finish": "2020-01-02T00:00:00Z", + }) + + // Inserting a row into the table fails because there is a reservation that overlaps with the new one. + MustNotInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "1", + "start": "2020-01-01T08:00:00Z", + "finish": "2020-01-02T08:00:00Z", + }, testutils.ExclusionViolationErrorCode) + + // Insert a row that overlaps with the existing one, but is canceled. + MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "1", + "start": "2020-01-01T08:00:00Z", + "finish": "2020-01-02T08:00:00Z", + "canceled": "true", + }) + + // Insert a row that overlaps with the existing one, but with a different id. + MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ + "id": "2", + "start": "2020-01-01T08:00:00Z", + "finish": "2020-01-02T08:00:00Z", + }) + }, + }, }) } diff --git a/pkg/sql2pgroll/create_table.go b/pkg/sql2pgroll/create_table.go index bb2204ffc..e1f36f422 100644 --- a/pkg/sql2pgroll/create_table.go +++ b/pkg/sql2pgroll/create_table.go @@ -4,6 +4,7 @@ package sql2pgroll import ( "fmt" + "strings" pgq "github.com/xataio/pg_query_go/v6" @@ -248,6 +249,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { var checkExpr string var err error var references *migrations.ConstraintReferences + var exclude *migrations.ConstraintExclude columns := make([]string, len(c.Keys)) for i, key := range c.Keys { @@ -307,6 +309,34 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { OnDeleteSetColumns: columnsToSet, OnUpdate: onUpdate, } + case pgq.ConstrType_CONSTR_EXCLUSION: + constraintType = migrations.ConstraintTypeExclude + exclude = &migrations.ConstraintExclude{ + IndexMethod: c.AccessMethod, + } + if c.GetWhereClause() != nil { + whereClause, err := pgq.DeparseExpr(c.GetWhereClause()) + if err != nil { + return nil, nil + } + exclude.Predicate = whereClause + } + exclusionElements := make([]string, len(c.Exclusions)) + for i, elem := range c.Exclusions { + if elem.GetList() == nil && len(elem.GetList().Items) != 2 { + return nil, nil + } + indexElem, err := pgq.DeparseIndexElem(elem.GetList().Items[0]) + if err != nil { + return nil, nil + } + anyOp, err := pgq.DeparseAnyOperator(elem.GetList().Items[1].GetList().Items) + if err != nil { + return nil, nil + } + exclusionElements[i] = fmt.Sprintf("%s WITH %s", indexElem, anyOp) + } + exclude.Elements = strings.Join(exclusionElements, ", ") default: return nil, nil } @@ -345,6 +375,7 @@ func convertConstraint(c *pgq.Constraint) (*migrations.Constraint, error) { IndexParameters: indexParameters, Check: checkExpr, References: references, + Exclude: exclude, }, nil } diff --git a/pkg/sql2pgroll/create_table_test.go b/pkg/sql2pgroll/create_table_test.go index 41f49824b..a88ec8cbc 100644 --- a/pkg/sql2pgroll/create_table_test.go +++ b/pkg/sql2pgroll/create_table_test.go @@ -213,6 +213,14 @@ func TestConvertCreateTableStatements(t *testing.T) { sql: "CREATE TABLE foo(a int, b int, c int, FOREIGN KEY (a, b, c) REFERENCES bar(d, e, f) MATCH FULL)", expectedOp: expect.CreateTableOp33, }, + { + sql: "CREATE TABLE foo(id integer, s timestamptz, e timestamptz, canceled boolean DEFAULT false, EXCLUDE USING gist (id WITH =, tstzrange(s, e) WITH &&) WHERE (not canceled))", + expectedOp: expect.CreateTableOp34, + }, + { + sql: "CREATE TABLE foo(id integer, b text, EXCLUDE USING btree (id WITH =) DEFERRABLE INITIALLY DEFERRED)", + expectedOp: expect.CreateTableOp35, + }, } for _, tc := range tests { diff --git a/pkg/sql2pgroll/expect/create_table.go b/pkg/sql2pgroll/expect/create_table.go index 05a17428d..3317f7164 100644 --- a/pkg/sql2pgroll/expect/create_table.go +++ b/pkg/sql2pgroll/expect/create_table.go @@ -635,3 +635,75 @@ var CreateTableOp33 = &migrations.OpCreateTable{ }, }, } + +var CreateTableOp34 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + Nullable: true, + }, + { + Name: "s", + Type: "timestamptz", + Nullable: true, + }, + { + Name: "e", + Type: "timestamptz", + Nullable: true, + }, + { + Name: "canceled", + Type: "boolean", + Nullable: true, + Default: ptr("false"), + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeExclude, + Columns: []string{}, + NullsNotDistinct: false, + Deferrable: false, + InitiallyDeferred: false, + NoInherit: false, + Exclude: &migrations.ConstraintExclude{ + IndexMethod: "gist", + Predicate: "NOT canceled", + Elements: "id WITH =, tstzrange(s, e) WITH &&", + }, + }, + }, +} + +var CreateTableOp35 = &migrations.OpCreateTable{ + Name: "foo", + Columns: []migrations.Column{ + { + Name: "id", + Type: "int", + Nullable: true, + }, + { + Name: "b", + Type: "text", + Nullable: true, + }, + }, + Constraints: []migrations.Constraint{ + { + Type: migrations.ConstraintTypeExclude, + Columns: []string{}, + NullsNotDistinct: false, + Deferrable: true, + InitiallyDeferred: true, + NoInherit: false, + Exclude: &migrations.ConstraintExclude{ + IndexMethod: "btree", + Elements: "id WITH =", + }, + }, + }, +} diff --git a/schema.json b/schema.json index 04e608983..df86113f1 100644 --- a/schema.json +++ b/schema.json @@ -354,6 +354,32 @@ }, "required": ["columns", "references"] } + }, + { + "if": { + "properties": { + "type": { + "const": "exclude" + } + } + }, + "then": { + "properties": { + "check": { + "const": "" + }, + "no_inherit": { + "const": false + }, + "nulls_not_distinct": { + "const": false + }, + "columns": { + "const": [] + } + }, + "required": ["exclude"] + } } ], "required": ["name", "type"], From 6c448e7865f957ca5b038d5b46ef8d429a4f1152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 12:10:36 +0100 Subject: [PATCH 52/66] even more --- docs/operations/create_table.mdx | 13 ++++- examples/.ledger | 1 + ...reate_table_with_exclusion_constraint.json | 56 +++++++++++++++++++ ...-invalid-exclusion-missing-exclusion.txtar | 34 +++++++++++ ...ble-17-invalid-exclusion-columns-set.txtar | 39 +++++++++++++ internal/testutils/util.go | 6 ++ 6 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 examples/52_create_table_with_exclusion_constraint.json create mode 100644 internal/jsonschema/testdata/create-table-16-invalid-exclusion-missing-exclusion.txtar create mode 100644 internal/jsonschema/testdata/create-table-17-invalid-exclusion-columns-set.txtar diff --git a/docs/operations/create_table.mdx b/docs/operations/create_table.mdx index c60d8cc56..50c4d45c2 100644 --- a/docs/operations/create_table.mdx +++ b/docs/operations/create_table.mdx @@ -76,10 +76,15 @@ Each `constraint` is defined as: "storage_parameters": "parameter=value", "include_columns": ["list", "of", "columns", "included in index"] }, + "exclude": { + "index_method": "name of the index method, e.g. btree", + "elements": "exclude elements", + "predicate": "WHERE clause of the exclude constraint" + } }, ``` -Supported constraint types: `unique`, `check`, `primary_key`, `foreign_key`. +Supported constraint types: `unique`, `check`, `primary_key`, `foreign_key`, `exclude`. Please note that you can only configure primary keys in `columns` list or `constraints` list, but not in both places. @@ -154,3 +159,9 @@ Create a table with table level constraints: Create a table with table level foreign key constraints: + +### Create a table with exclusion constraint + +Create a table with an exclusion: + + diff --git a/examples/.ledger b/examples/.ledger index 7003ffe5b..f7afa5b18 100644 --- a/examples/.ledger +++ b/examples/.ledger @@ -49,3 +49,4 @@ 49_unset_not_null_on_indexed_column.json 50_create_table_with_table_constraint.json 51_create_table_with_table_foreign_key_constraint.json +52_create_table_with_exclusion_constraint.json diff --git a/examples/52_create_table_with_exclusion_constraint.json b/examples/52_create_table_with_exclusion_constraint.json new file mode 100644 index 000000000..9a657cf89 --- /dev/null +++ b/examples/52_create_table_with_exclusion_constraint.json @@ -0,0 +1,56 @@ +{ + "name": "52_create_table_with_table_exclusion_constraint", + "operations": [ + { + "create_table": { + "name": "room_reservations", + "columns": [ + { + "name": "id", + "type": "serial" + }, + { + "name": "start", + "type": "timestamp" + }, + { + "name": "name", + "type": "text" + }, + { + "name": "end", + "type": "timestamp" + }, + { + "name": "canceled", + "type": "boolean", + "default": "false" + } + ], + "constraints": [ + { + "name": "rooms_pk", + "type": "primary_key", + "columns": [ + "id" + ] + }, + { + "name": "forbid_double_booking", + "type": "exclude", + "exclude": { + "index_method": "gist", + "elements": "id WITH =, tsrange(\"start\", \"end\") WITH &&", + "predicate": "NOT canceled" + } + }, + { + "name": "name_must_be_present", + "type": "check", + "check": "length(name) > 0" + } + ] + } + } + ] +} diff --git a/internal/jsonschema/testdata/create-table-16-invalid-exclusion-missing-exclusion.txtar b/internal/jsonschema/testdata/create-table-16-invalid-exclusion-missing-exclusion.txtar new file mode 100644 index 000000000..9eb9900ec --- /dev/null +++ b/internal/jsonschema/testdata/create-table-16-invalid-exclusion-missing-exclusion.txtar @@ -0,0 +1,34 @@ +This is an invalid 'create_table' migration. +Exclusion constraints must have exclude configured + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_fk", + "type": "exclude" + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file diff --git a/internal/jsonschema/testdata/create-table-17-invalid-exclusion-columns-set.txtar b/internal/jsonschema/testdata/create-table-17-invalid-exclusion-columns-set.txtar new file mode 100644 index 000000000..d31279c5d --- /dev/null +++ b/internal/jsonschema/testdata/create-table-17-invalid-exclusion-columns-set.txtar @@ -0,0 +1,39 @@ +This is an invalid 'create_table' migration. +Exclusion constraints mustn't have columns configured + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "create_table": { + "name": "posts", + "columns": [ + { + "name": "title", + "type": "varchar(255)" + }, + { + "name": "user_id", + "type": "integer", + "nullable": true + } + ], + "constraints": [ + { + "name": "my_invalid_exclusion", + "type": "exclude", + "columns": ["invalid"], + "exclude": { + "index_method": "btree", + "elements": "title WITH =" + } + } + ] + } + } + ] +} + +-- valid -- +false \ No newline at end of file diff --git a/internal/testutils/util.go b/internal/testutils/util.go index 5dc45b247..aa9f8ff95 100644 --- a/internal/testutils/util.go +++ b/internal/testutils/util.go @@ -238,6 +238,12 @@ func WithMigratorAndStateAndConnectionToContainerWithOptions(t *testing.T, opts t.Fatal(err) } + // add extension for btree_gist + _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION btree_gist SCHEMA %s", schema)) + if err != nil { + t.Fatal(err) + } + fn(mig, st, db) } From 8e060ce511de84be420f55ced7e13d521f32a13d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 12:16:16 +0100 Subject: [PATCH 53/66] fix test --- internal/testutils/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testutils/util.go b/internal/testutils/util.go index aa9f8ff95..bbf00a642 100644 --- a/internal/testutils/util.go +++ b/internal/testutils/util.go @@ -239,7 +239,7 @@ func WithMigratorAndStateAndConnectionToContainerWithOptions(t *testing.T, opts } // add extension for btree_gist - _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION btree_gist SCHEMA %s", schema)) + _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION btree_gist SCHEMA %s", "public")) if err != nil { t.Fatal(err) } From 2c387fbcb11054eef87d6f0fbbe2e8a320c3d82d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 12:48:15 +0100 Subject: [PATCH 54/66] add missing extension --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 19ce0dff6..1b72b4714 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -218,7 +218,7 @@ jobs: - name: Run example migrations run: | if [ "$PGROLL_SCHEMA" != "public" ]; then - psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE SCHEMA $PGROLL_SCHEMA;" + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE SCHEMA $PGROLL_SCHEMA; CREATE EXTENSION btree_gist SCHEMA $PGROLL_SCHEMA;" fi make examples From a1413339d4ab8045169406d13e17d163ff3a001c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 12:53:21 +0100 Subject: [PATCH 55/66] always extension --- .github/workflows/build.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1b72b4714..398eb76de 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -218,8 +218,9 @@ jobs: - name: Run example migrations run: | if [ "$PGROLL_SCHEMA" != "public" ]; then - psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE SCHEMA $PGROLL_SCHEMA; CREATE EXTENSION btree_gist SCHEMA $PGROLL_SCHEMA;" + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE SCHEMA $PGROLL_SCHEMA;" fi + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE EXTENSION btree_gist SCHEMA $PGROLL_SCHEMA;" make examples env: From c1e11cebda296f0bbfca3f81307ff6ab0ea5dfbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 12:55:33 +0100 Subject: [PATCH 56/66] install extension --- .github/workflows/build.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 398eb76de..e820b034f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -56,7 +56,11 @@ jobs: go-version-file: 'go.mod' - name: Run tests - run: go test ./... + run: | + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE EXTENSION btree_gist SCHEMA $PGROLL_SCHEMA;" + + go test ./... + env: POSTGRES_VERSION: ${{ matrix.pgVersion }} PGROLL_TEST_SCHEMA: ${{ matrix.testSchema }} From e57c36136ee2071804e50133e9604b7cf4a0d365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 12:59:21 +0100 Subject: [PATCH 57/66] most jo lesz? --- .github/workflows/build.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e820b034f..1c68907bc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -62,6 +62,9 @@ jobs: go test ./... env: + POSTGRES_PORT: 5432 + POSTGRES_HOST: localhost + PGPASSWORD: postgres POSTGRES_VERSION: ${{ matrix.pgVersion }} PGROLL_TEST_SCHEMA: ${{ matrix.testSchema }} From 5bf85b6e2415b5a15c50c0662f1494ed8346f8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 13:05:04 +0100 Subject: [PATCH 58/66] miert --- .github/workflows/build.yml | 9 +-------- internal/testutils/util.go | 3 ++- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1c68907bc..398eb76de 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -56,15 +56,8 @@ jobs: go-version-file: 'go.mod' - name: Run tests - run: | - psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE EXTENSION btree_gist SCHEMA $PGROLL_SCHEMA;" - - go test ./... - + run: go test ./... env: - POSTGRES_PORT: 5432 - POSTGRES_HOST: localhost - PGPASSWORD: postgres POSTGRES_VERSION: ${{ matrix.pgVersion }} PGROLL_TEST_SCHEMA: ${{ matrix.testSchema }} diff --git a/internal/testutils/util.go b/internal/testutils/util.go index bbf00a642..8c2f8804b 100644 --- a/internal/testutils/util.go +++ b/internal/testutils/util.go @@ -188,10 +188,11 @@ func WithMigratorInSchemaAndConnectionToContainerWithOptions(t testing.TB, schem } // add extension for btree_gist - _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION btree_gist SCHEMA %s", schema)) + _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS btree_gist SCHEMA %s", schema)) if err != nil { t.Fatal(err) } + fmt.Println("Created extension btree_gist") fn(mig, db) } From e30a35e0caa285346ec970d66d4a702f13703461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 13:13:49 +0100 Subject: [PATCH 59/66] clean --- .github/workflows/build.yml | 2 +- internal/testutils/util.go | 3 +-- pkg/migrations/op_create_table.go | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 398eb76de..a2f460e55 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -220,7 +220,7 @@ jobs: if [ "$PGROLL_SCHEMA" != "public" ]; then psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE SCHEMA $PGROLL_SCHEMA;" fi - psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE EXTENSION btree_gist SCHEMA $PGROLL_SCHEMA;" + psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE EXTENSION IF NOT EXISTS btree_gist SCHEMA $PGROLL_SCHEMA;" make examples env: diff --git a/internal/testutils/util.go b/internal/testutils/util.go index 8c2f8804b..98691b19e 100644 --- a/internal/testutils/util.go +++ b/internal/testutils/util.go @@ -192,7 +192,6 @@ func WithMigratorInSchemaAndConnectionToContainerWithOptions(t testing.TB, schem if err != nil { t.Fatal(err) } - fmt.Println("Created extension btree_gist") fn(mig, db) } @@ -240,7 +239,7 @@ func WithMigratorAndStateAndConnectionToContainerWithOptions(t *testing.T, opts } // add extension for btree_gist - _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION btree_gist SCHEMA %s", "public")) + _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS btree_gist SCHEMA %s", "public")) if err != nil { t.Fatal(err) } diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index a780ac596..fb82968d8 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -27,8 +27,6 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri if err != nil { return nil, fmt.Errorf("failed to create constraints SQL: %w", err) } - fmt.Println(columnsSQL) - fmt.Println(constraintsSQL) // Create the table _, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s %s)", From 7004ff574e12b0a9f68f703dcb529ae6b88b55c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 13:26:53 +0100 Subject: [PATCH 60/66] move --- internal/testutils/util.go | 12 ------------ pkg/migrations/op_common_test.go | 7 +++++++ pkg/migrations/op_create_table_test.go | 3 ++- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/internal/testutils/util.go b/internal/testutils/util.go index 98691b19e..4893878a4 100644 --- a/internal/testutils/util.go +++ b/internal/testutils/util.go @@ -187,12 +187,6 @@ func WithMigratorInSchemaAndConnectionToContainerWithOptions(t testing.TB, schem t.Fatal(err) } - // add extension for btree_gist - _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS btree_gist SCHEMA %s", schema)) - if err != nil { - t.Fatal(err) - } - fn(mig, db) } @@ -238,12 +232,6 @@ func WithMigratorAndStateAndConnectionToContainerWithOptions(t *testing.T, opts t.Fatal(err) } - // add extension for btree_gist - _, err = db.ExecContext(ctx, fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS btree_gist SCHEMA %s", "public")) - if err != nil { - t.Fatal(err) - } - fn(mig, st, db) } diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 6f81f7bd2..495f3a2b2 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -25,6 +25,7 @@ import ( type TestCase struct { name string minPgMajorVersion int + pgExtensions []string migrations []migrations.Migration wantStartErr error wantRollbackErr error @@ -52,6 +53,12 @@ func ExecuteTests(t *testing.T, tests TestCases, opts ...roll.Option) { testutils.WithMigratorInSchemaAndConnectionToContainerWithOptions(t, testSchema, opts, func(mig *roll.Roll, db *sql.DB) { ctx := context.Background() + if tt.pgExtensions != nil { + for _, ext := range tt.pgExtensions { + db.Exec(fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s SCHEMA %s", ext, testSchema)) + } + } + // run all migrations except the last one for i := 0; i < len(tt.migrations)-1; i++ { if err := mig.Start(ctx, &tt.migrations[i]); err != nil { diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index a377dee7b..32a072f99 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -1091,7 +1091,8 @@ func TestCreateTable(t *testing.T) { }, }, { - name: "create table with an exclude constraint to check overlap", + name: "create table with an exclude constraint to check overlap", + pgExtensions: []string{"btree_gist"}, migrations: []migrations.Migration{ { Name: "01_create_table", From d9e51b5283bf05fa1e6f5cdca7b0b3107daaa656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 13:31:16 +0100 Subject: [PATCH 61/66] update dep --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 2673f0808..705e5419d 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/stretchr/testify v1.10.0 github.com/testcontainers/testcontainers-go v0.35.0 github.com/testcontainers/testcontainers-go/modules/postgres v0.35.0 - github.com/xataio/pg_query_go/v6 v6.0.0-20250123182324-526a22cbe0c0 + github.com/xataio/pg_query_go/v6 v6.0.0-20250124115938-4fa82ad6d036 golang.org/x/tools v0.29.0 ) diff --git a/go.sum b/go.sum index 8b390327f..7e25b7807 100644 --- a/go.sum +++ b/go.sum @@ -225,6 +225,8 @@ github.com/xataio/pg_query_go/v6 v6.0.0-20250122133641-54118c062181 h1:iLOHgul20 github.com/xataio/pg_query_go/v6 v6.0.0-20250122133641-54118c062181/go.mod h1:GK6bpfAhPtZb7wG/IccqvnH+cz3cmvvRTkC+MosESGo= github.com/xataio/pg_query_go/v6 v6.0.0-20250123182324-526a22cbe0c0 h1:/fXLU7NusFv8TSEAM7n+vtsN5Rr2La9tXWEunrJOWrI= github.com/xataio/pg_query_go/v6 v6.0.0-20250123182324-526a22cbe0c0/go.mod h1:GK6bpfAhPtZb7wG/IccqvnH+cz3cmvvRTkC+MosESGo= +github.com/xataio/pg_query_go/v6 v6.0.0-20250124115938-4fa82ad6d036 h1:QMjBW1XIFUDzyUs/zlYmUo8lNO+hQ4VVt0msFv2AYpw= +github.com/xataio/pg_query_go/v6 v6.0.0-20250124115938-4fa82ad6d036/go.mod h1:GK6bpfAhPtZb7wG/IccqvnH+cz3cmvvRTkC+MosESGo= github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778/go.mod h1:2MuV+tbUrU1zIOPMxZ5EncGwgmMJsa+9ucAQZXxsObs= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= From 76122d2b4672a8e317fb68ebafcf3103d4955c3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Fri, 24 Jan 2025 13:36:29 +0100 Subject: [PATCH 62/66] more --- pkg/migrations/types.go | 5 +++-- pkg/schema/schema.go | 2 +- schema.json | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 1e6217eb4..762fbe41a 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -81,7 +81,7 @@ type Constraint struct { // Deferable constraint Deferrable bool `json:"deferrable,omitempty"` - // Exclude corresponds to the JSON schema field "exclude". + // Exclude constraint definition Exclude *ConstraintExclude `json:"exclude,omitempty"` // IndexParameters corresponds to the JSON schema field "index_parameters". @@ -106,8 +106,9 @@ type Constraint struct { Type ConstraintType `json:"type"` } +// Exclude constraint definition type ConstraintExclude struct { - // Columns of the exclude constraint + // Expressions of the exclude constraint Elements string `json:"elements,omitempty"` // Index method diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 4b7bce6fe..7e1d5259a 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -157,7 +157,7 @@ type UniqueConstraint struct { Columns []string `json:"columns"` } -// UniqueConstraint represents a unique constraint on a table +// ExcludeConstraint represents a unique constraint on a table type ExcludeConstraint struct { // Name is the name of the exclude constraint in postgres Name string `json:"name"` diff --git a/schema.json b/schema.json index df86113f1..d56be702c 100644 --- a/schema.json +++ b/schema.json @@ -217,6 +217,7 @@ "exclude": { "type": "object", "additionalProperties": false, + "description": "Exclude constraint definition", "properties": { "index_method": { "description": "Index method", @@ -226,7 +227,7 @@ "elements": { "type": "string", "default": "", - "description": "Columns of the exclude constraint" + "description": "Expressions of the exclude constraint" }, "predicate": { "type": "string", From 3a906842a9669220f4c5b6f34e5e950682278de3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 27 Jan 2025 13:05:28 +0100 Subject: [PATCH 63/66] rm extension --- .github/workflows/build.yml | 1 - ...reate_table_with_exclusion_constraint.json | 26 ++-- pkg/migrations/op_common_test.go | 7 -- pkg/migrations/op_create_table_test.go | 115 ------------------ 4 files changed, 8 insertions(+), 141 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a2f460e55..19ce0dff6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -220,7 +220,6 @@ jobs: if [ "$PGROLL_SCHEMA" != "public" ]; then psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE SCHEMA $PGROLL_SCHEMA;" fi - psql -h $POSTGRES_HOST -p $POSTGRES_PORT -U postgres -c "CREATE EXTENSION IF NOT EXISTS btree_gist SCHEMA $PGROLL_SCHEMA;" make examples env: diff --git a/examples/52_create_table_with_exclusion_constraint.json b/examples/52_create_table_with_exclusion_constraint.json index 9a657cf89..a6a6ca963 100644 --- a/examples/52_create_table_with_exclusion_constraint.json +++ b/examples/52_create_table_with_exclusion_constraint.json @@ -3,14 +3,14 @@ "operations": [ { "create_table": { - "name": "room_reservations", + "name": "employees", "columns": [ { "name": "id", "type": "serial" }, { - "name": "start", + "name": "start_time", "type": "timestamp" }, { @@ -18,13 +18,8 @@ "type": "text" }, { - "name": "end", - "type": "timestamp" - }, - { - "name": "canceled", - "type": "boolean", - "default": "false" + "name": "position", + "type": "text" } ], "constraints": [ @@ -36,18 +31,13 @@ ] }, { - "name": "forbid_double_booking", + "name": "forbid_duplicated names", "type": "exclude", "exclude": { - "index_method": "gist", - "elements": "id WITH =, tsrange(\"start\", \"end\") WITH &&", - "predicate": "NOT canceled" + "index_method": "btree", + "elements": "name WITH =", + "predicate": "name IS NOT NULL" } - }, - { - "name": "name_must_be_present", - "type": "check", - "check": "length(name) > 0" } ] } diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 495f3a2b2..6f81f7bd2 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -25,7 +25,6 @@ import ( type TestCase struct { name string minPgMajorVersion int - pgExtensions []string migrations []migrations.Migration wantStartErr error wantRollbackErr error @@ -53,12 +52,6 @@ func ExecuteTests(t *testing.T, tests TestCases, opts ...roll.Option) { testutils.WithMigratorInSchemaAndConnectionToContainerWithOptions(t, testSchema, opts, func(mig *roll.Roll, db *sql.DB) { ctx := context.Background() - if tt.pgExtensions != nil { - for _, ext := range tt.pgExtensions { - db.Exec(fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s SCHEMA %s", ext, testSchema)) - } - } - // run all migrations except the last one for i := 0; i < len(tt.migrations)-1; i++ { if err := mig.Start(ctx, &tt.migrations[i]); err != nil { diff --git a/pkg/migrations/op_create_table_test.go b/pkg/migrations/op_create_table_test.go index 32a072f99..4f5bb31ec 100644 --- a/pkg/migrations/op_create_table_test.go +++ b/pkg/migrations/op_create_table_test.go @@ -1090,121 +1090,6 @@ func TestCreateTable(t *testing.T) { }, testutils.ExclusionViolationErrorCode) }, }, - { - name: "create table with an exclude constraint to check overlap", - pgExtensions: []string{"btree_gist"}, - migrations: []migrations.Migration{ - { - Name: "01_create_table", - Operations: migrations.Operations{ - &migrations.OpCreateTable{ - Name: "reservations", - Columns: []migrations.Column{ - { - Name: "id", - Type: "int", - }, - { - Name: "start", - Type: "timestamp", - }, - { - Name: "finish", - Type: "timestamp", - }, - { - Name: "canceled", - Type: "bool", - Default: ptr("false"), - }, - }, - Constraints: []migrations.Constraint{ - { - Name: "forbid_reservation_overlap", - Type: migrations.ConstraintTypeExclude, - Exclude: &migrations.ConstraintExclude{ - IndexMethod: "gist", - Elements: "id WITH =, tsrange(start, finish) WITH &&", - Predicate: "NOT canceled", - }, - }, - }, - }, - }, - }, - }, - afterStart: func(t *testing.T, db *sql.DB, schema string) { - // The exclusion constraint exists on the new table. - ExcludeConstraintMustExist(t, db, schema, "reservations", "forbid_reservation_overlap") - - // Inserting a row into the table succeeds when the exclude constraint is satisfied. - // This is the first row, so the constraint is satisfied. - MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "1", - "start": "2020-01-01T00:00:00Z", - "finish": "2020-01-02T00:00:00Z", - }) - - // Inserting a row into the table fails because there is a reservation that overlaps with the new one. - MustNotInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "1", - "start": "2020-01-01T08:00:00Z", - "finish": "2020-01-02T08:00:00Z", - }, testutils.ExclusionViolationErrorCode) - - // Insert a row that overlaps with the existing one, but is canceled. - MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "1", - "start": "2020-01-01T08:00:00Z", - "finish": "2020-01-02T08:00:00Z", - "canceled": "true", - }) - - // Insert a row that overlaps with the existing one, but with a different id. - MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "2", - "start": "2020-01-01T08:00:00Z", - "finish": "2020-01-02T08:00:00Z", - }) - }, - afterRollback: func(t *testing.T, db *sql.DB, schema string) { - // The table has been dropped, so the constraint is gone. - }, - afterComplete: func(t *testing.T, db *sql.DB, schema string) { - // The exclusion constraint exists on the new table. - ExcludeConstraintMustExist(t, db, schema, "reservations", "forbid_reservation_overlap") - - // Inserting a row into the table succeeds when the exclude constraint is satisfied. - // This is the first row, so the constraint is satisfied. - MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "1", - "start": "2020-01-01T00:00:00Z", - "finish": "2020-01-02T00:00:00Z", - }) - - // Inserting a row into the table fails because there is a reservation that overlaps with the new one. - MustNotInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "1", - "start": "2020-01-01T08:00:00Z", - "finish": "2020-01-02T08:00:00Z", - }, testutils.ExclusionViolationErrorCode) - - // Insert a row that overlaps with the existing one, but is canceled. - MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "1", - "start": "2020-01-01T08:00:00Z", - "finish": "2020-01-02T08:00:00Z", - "canceled": "true", - }) - - // Insert a row that overlaps with the existing one, but with a different id. - MustInsert(t, db, schema, "01_create_table", "reservations", map[string]string{ - "id": "2", - "start": "2020-01-01T08:00:00Z", - "finish": "2020-01-02T08:00:00Z", - }) - }, - }, }) } From 1d301bb536a38ca288c7f90836fccdf5f941f248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 27 Jan 2025 13:07:55 +0100 Subject: [PATCH 64/66] fix name --- examples/52_create_table_with_exclusion_constraint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/52_create_table_with_exclusion_constraint.json b/examples/52_create_table_with_exclusion_constraint.json index a6a6ca963..1968a78e5 100644 --- a/examples/52_create_table_with_exclusion_constraint.json +++ b/examples/52_create_table_with_exclusion_constraint.json @@ -31,7 +31,7 @@ ] }, { - "name": "forbid_duplicated names", + "name": "forbid_duplicated_names", "type": "exclude", "exclude": { "index_method": "btree", From b943ed5a7fc289ee1a3da38683e12bfadf761c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 27 Jan 2025 13:12:05 +0100 Subject: [PATCH 65/66] rename example table --- examples/52_create_table_with_exclusion_constraint.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/52_create_table_with_exclusion_constraint.json b/examples/52_create_table_with_exclusion_constraint.json index 1968a78e5..a68866370 100644 --- a/examples/52_create_table_with_exclusion_constraint.json +++ b/examples/52_create_table_with_exclusion_constraint.json @@ -3,22 +3,22 @@ "operations": [ { "create_table": { - "name": "employees", + "name": "library", "columns": [ { "name": "id", "type": "serial" }, { - "name": "start_time", + "name": "returned", "type": "timestamp" }, { - "name": "name", + "name": "title", "type": "text" }, { - "name": "position", + "name": "summary", "type": "text" } ], From 9e5d6a4448672ebe816fe43979a5cadac930ac60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mi=20V=C3=A1nyi?= Date: Mon, 27 Jan 2025 16:10:29 +0100 Subject: [PATCH 66/66] really fix example --- examples/52_create_table_with_exclusion_constraint.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/52_create_table_with_exclusion_constraint.json b/examples/52_create_table_with_exclusion_constraint.json index a68866370..78cb12340 100644 --- a/examples/52_create_table_with_exclusion_constraint.json +++ b/examples/52_create_table_with_exclusion_constraint.json @@ -31,12 +31,12 @@ ] }, { - "name": "forbid_duplicated_names", + "name": "forbid_duplicated_titles", "type": "exclude", "exclude": { "index_method": "btree", - "elements": "name WITH =", - "predicate": "name IS NOT NULL" + "elements": "title WITH =", + "predicate": "title IS NOT NULL" } } ]