Improve validation of create_table and add_column operations (#666)

Improve validation of `create_table` and `add_column` operations.

Previously, these operations would fail at migration `Start` if one of
the column definitions was invalid due to having a missing `name` or
`type` field.

This PR adds extra validation checks to both operations to ensure that
such migrations fail at the validation stage instead.

Author: andrew-farries

pkg/migrations/column.go

@@ -26,3 +26,15 @@ func (c *Column) HasImplicitDefault() bool {
 		return false
 	}
 }
+
+// Validate returns true iff the column contains all fields required to create
+// the column
+func (c *Column) Validate() bool {
+	if c.Name == "" {
+		return false
+	}
+	if c.Type == "" {
+		return false
+	}
+	return true
+}

pkg/migrations/errors.go

@@ -41,6 +41,16 @@ type ColumnAlreadyExistsError struct {
 	Name  string
 }
 
+type ColumnIsInvalidError struct {
+	Table string
+	Name  string
+}
+
+func (e ColumnIsInvalidError) Error() string {
+	return fmt.Sprintf(`column %q in table %q is missing one or more required fields - "name" or "type"`,
+		e.Name, e.Table)
+}
+
 func (e ColumnAlreadyExistsError) Error() string {
 	return fmt.Sprintf("column %q already exists in table %q", e.Name, e.Table)
 }

pkg/migrations/op_add_column.go

@@ -148,6 +148,11 @@ func (o *OpAddColumn) Validate(ctx context.Context, s *schema.Schema) error {
 		return err
 	}
 
+	// Validate that the column contains all required fields
+	if !o.Column.Validate() {
+		return ColumnIsInvalidError{Table: o.Table, Name: o.Column.Name}
+	}
+
 	table := s.GetTable(o.Table)
 	if table == nil {
 		return TableDoesNotExistError{Name: o.Table}

pkg/migrations/op_add_column_test.go

@@ -1322,6 +1322,44 @@ func TestAddColumnValidation(t *testing.T) {
 			},
 			wantStartErr: migrations.ColumnAlreadyExistsError{Table: "users", Name: "name"},
 		},
+		{
+			name: "column must be valid (not missing its type)",
+			migrations: []migrations.Migration{
+				addTableMigration,
+				{
+					Name: "02_add_column",
+					Operations: migrations.Operations{
+						&migrations.OpAddColumn{
+							Table: "users",
+							Column: migrations.Column{
+								Name: "description",
+								// Missing type
+							},
+						},
+					},
+				},
+			},
+			wantStartErr: migrations.ColumnIsInvalidError{Table: "users", Name: "description"},
+		},
+		{
+			name: "column must be valid (not missing its name)",
+			migrations: []migrations.Migration{
+				addTableMigration,
+				{
+					Name: "02_add_column",
+					Operations: migrations.Operations{
+						&migrations.OpAddColumn{
+							Table: "users",
+							Column: migrations.Column{
+								// Missing name
+								Type: "text",
+							},
+						},
+					},
+				},
+			},
+			wantStartErr: migrations.ColumnIsInvalidError{Table: "users", Name: ""},
+		},
 		{
 			name: "up SQL is mandatory when adding a NOT NULL column with no DEFAULT",
 			migrations: []migrations.Migration{

pkg/migrations/op_create_table.go

@@ -89,6 +89,11 @@ func (o *OpCreateTable) Validate(ctx context.Context, s *schema.Schema) error {
 			return fmt.Errorf("invalid column: %w", err)
 		}
 
+		// Validate that the column contains all required fields
+		if !col.Validate() {
+			return ColumnIsInvalidError{Table: o.Name, Name: col.Name}
+		}
+
 		// Ensure that any foreign key references are valid, ie. the referenced
 		// table and column exist.
 		if col.References != nil {

pkg/migrations/op_create_table_test.go

@@ -1323,6 +1323,56 @@ func TestCreateTableValidation(t *testing.T) {
 			},
 			wantStartErr: migrations.InvalidIdentifierLengthError{Name: invalidName},
 		},
+		{
+			name: "column definition missing a name",
+			migrations: []migrations.Migration{
+				{
+					Name: "01_create_table",
+					Operations: migrations.Operations{
+						&migrations.OpCreateTable{
+							Name: "orders",
+							Columns: []migrations.Column{
+								{
+									Name: "id",
+									Type: "serial",
+									Pk:   true,
+								},
+								{
+									// missing name
+									Type: "varchar(255)",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantStartErr: migrations.ColumnIsInvalidError{Table: "orders", Name: ""},
+		},
+		{
+			name: "column definition missing a type",
+			migrations: []migrations.Migration{
+				{
+					Name: "01_create_table",
+					Operations: migrations.Operations{
+						&migrations.OpCreateTable{
+							Name: "orders",
+							Columns: []migrations.Column{
+								{
+									Name: "id",
+									Type: "serial",
+									Pk:   true,
+								},
+								{
+									Name: "name",
+									// missing type
+								},
+							},
+						},
+					},
+				},
+			},
+			wantStartErr: migrations.ColumnIsInvalidError{Table: "orders", Name: "name"},
+		},
 		{
 			name: "invalid column name",
 			migrations: []migrations.Migration{