Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure each operation type is aware of any preceding column or table renames #239

Closed
andrew-farries opened this issue Jan 16, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@andrew-farries
Copy link
Collaborator

andrew-farries commented Jan 16, 2024

Ensure that each operation type is aware of any preceding column or table renames. In a multi-operation migration, tables and columns may be renamed by a preceding operation in the same migration.

Tasks

operation status notes / assignee PRs
op_add_column 🟢 #590
op_alter_column (rename only) 🟢 #593
op_change_type 🟢 #661
op_create_constraint 🟢 #674, #676, #682
op_create_index 🟢 #662
op_create_table 🟢 (mulitiple other ops + #596)
op_drop_column 🟢 #591
op_drop_constraint n/a deprecated operation (#489)
op_drop_index 🟢 nothing to do
op_drop_multicolumn_constraint 🟢 #684
op_drop_not_null 🟢 #239
op_drop_table 🟢 #587, #589
op_raw_sql n/a
op_rename_constraint 🟢 no-op. deprecated operation #686
op_rename_table 🟢 #587
op_set_check 🟢 #622
op_set_comment n/a deprecated operation (#663)
op_set_default 🟢 #616
op_set_fk 🟢 #621
op_set_notnull 🟢 #606, #607, #608
op_set_replica_identity n/a deprecated operation
op_set_unique 🟢 #609

For each operation op, add test coverage to ensure that Start, Rollback , and Validate:

  • update the virtual schema
  • are aware of any indirections in the virtual schema when using table and column names

Notes

Handling multi-operation migrations that require multiple different backfills on the same table are out-of-scope for this issue. This issue is primarily about tracking table/column additions, renames and removals in the virtual schema between operations during migration validation, start and rollback.

@andrew-farries
Copy link
Collaborator Author

This was raised in the specific case of creating a table then adding an index to it in #203.

@exekias exekias added the enhancement New feature or request label Jan 18, 2024
@andrew-farries andrew-farries added this to the v1 milestone Apr 7, 2024
@kvch kvch assigned kvch and unassigned kvch Oct 16, 2024
@kvch kvch self-assigned this Oct 25, 2024
andrew-farries added a commit that referenced this issue Nov 6, 2024
Add a `--skip-validation` flag to the `start` command.

If set, migration validation is skipped.

For example, this table has a `NOT NULL` `name` field:

```json
{
  "name": "01_create_table",
  "operations": [
    {
      "create_table": {
        "name": "products",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "nullable": false
          }
        ]
      }
    }
  ]
}
```

starting this migration will fail as the `name` field is already `NOT
NULL`:

```json
{
  "name": "02_set_nullable",
  "operations": [
    {
      "alter_column": {
        "table": "products",
        "column": "name",
        "nullable": false,
        "up": "name",
        "down": "name"
      }
    }
  ]
}
```

```
Error: migration is invalid: column "name" on table "products" is NOT NULL
```

But if the start command is invoked with `--skip-validation` then the
start command succeeds.

This is part of #239
andrew-farries added a commit that referenced this issue Nov 6, 2024
Allow the `add_column` operation to add a column to a table that was
created by an operation earlier in the same migration.

The following migration would previously have failed to start:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "add_column": {
        "table": "players",
        "column": {
          "name": "rating",
          "type": "integer",
          "comment": "hello world",
          "check": {
            "name": "rating_check",
            "constraint": "rating > 0 AND rating < 100"
          },
          "nullable": false
        }
      }
    }
  ]
}
```

As of this PR, the migration can be started.

The above migration does not validate yet, but it can be started
successfully with the `--skip-validation` flag to the `start` command.

Part of #239
andrew-farries added a commit that referenced this issue Nov 6, 2024
Allow the `create_index` operation to add an index to a table that was
created by an operation earlier in the same migration.

The following migration would previously have failed to start:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "create_index": {
        "name": "idx_player_name",
        "table": "players",
        "columns": [
          "name"
        ]
      }
    }
  ]
}
```

As of this PR the migration can be started.

The above migration does not validate yet, but it can be started
successfully with the --skip-validation flag to the start command.

Part of #239
@andrew-farries andrew-farries self-assigned this Nov 7, 2024
andrew-farries added a commit that referenced this issue Nov 7, 2024
Allow the `create_index` operation to add an index to a column that was
created by an operation earlier in the same migration.

The following migration would previously have failed to start:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "add_column": {
        "table": "players",
        "column": {
          "name": "rating",
          "type": "integer",
          "comment": "hello world",
          "check": {
            "name": "rating_check",
            "constraint": "rating > 0 AND rating < 100"
          },
          "nullable": false
        }
      }
    },
    {
      "create_index": {
        "name": "idx_player_rating",
        "table": "players",
        "columns": [
          "rating"
        ]
      }
    }
  ]
}
```

As of this PR the migration can be started.

The above migration does not validate yet, but it can be started
successfully with the `--skip-validation` flag to the `start` command.

Part of #239
andrew-farries added a commit that referenced this issue Nov 8, 2024
Update the schema during `create_table` operation validation so that
validation of subsequent operations in the same migration can see the
new table.

This means that migrations that add a new table and then perform some
other operation, like adding a column, on that table can be validated
because the new table is visible to the `add_column` operation during
it's validation phase.

This means that the following migration is now able to validate:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "add_column": {
        "table": "players",
        "column": {
          "name": "rating",
          "type": "integer",
          "comment": "hello world",
          "check": {
            "name": "rating_check",
            "constraint": "rating > 0 AND rating < 100"
          },
          "nullable": false
        }
      }
    }
  ]
}
```

Previously, the new table would not have been visible to the
`add_column` operation and its validation would have failed.

In order to make this work the schema needs to be re-read from the
target database in between validation and migration start, as the
previous assumption that validation would not make changes to the
in-memory schema representation no longer holds.

Part of #239
andrew-farries added a commit that referenced this issue Nov 8, 2024
Update the schema during `add_column` operation validation so that
validation of subsequent operations in the same migration can see the
new column.

This means that migrations that add a new column and then perform some
other operation, like adding an index, on that column can be validated
because the new column is visible to the `create_index` operation during
it's validation phase.

This means that the following migration is now able to validate:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "add_column": {
        "table": "players",
        "column": {
          "name": "rating",
          "type": "integer",
          "comment": "hello world",
          "check": {
            "name": "rating_check",
            "constraint": "rating > 0 AND rating < 100"
          },
          "nullable": false
        }
      }
    },
    {
      "create_index": {
        "name": "idx_player_rating",
        "table": "players",
        "columns": [
          "rating"
        ]
      }
    }
  ]
}
```

Previously, the new column would not have been visible to the
`create_index` operation and its validation would have failed.

This PR does for the `add_column` operation what #455 did for the
`create_table` operation.

Part of #239
kvch pushed a commit to kvch/pgroll that referenced this issue Nov 11, 2024
…o#449)

Allow the `add_column` operation to add a column to a table that was
created by an operation earlier in the same migration.

The following migration would previously have failed to start:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "add_column": {
        "table": "players",
        "column": {
          "name": "rating",
          "type": "integer",
          "comment": "hello world",
          "check": {
            "name": "rating_check",
            "constraint": "rating > 0 AND rating < 100"
          },
          "nullable": false
        }
      }
    }
  ]
}
```

As of this PR, the migration can be started.

The above migration does not validate yet, but it can be started
successfully with the `--skip-validation` flag to the `start` command.

Part of xataio#239
kvch pushed a commit to kvch/pgroll that referenced this issue Nov 11, 2024
…o#451)

Allow the `create_index` operation to add an index to a table that was
created by an operation earlier in the same migration.

The following migration would previously have failed to start:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "create_index": {
        "name": "idx_player_name",
        "table": "players",
        "columns": [
          "name"
        ]
      }
    }
  ]
}
```

As of this PR the migration can be started.

The above migration does not validate yet, but it can be started
successfully with the --skip-validation flag to the start command.

Part of xataio#239
kvch pushed a commit to kvch/pgroll that referenced this issue Nov 11, 2024
…#454)

Allow the `create_index` operation to add an index to a column that was
created by an operation earlier in the same migration.

The following migration would previously have failed to start:

```json
{
  "name": "43_multiple_ops",
  "operations": [
    {
      "create_table": {
        "name": "players",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)",
            "check": {
              "name": "name_length_check",
              "constraint": "length(name) > 2"
            }
          }
        ]
      }
    },
    {
      "add_column": {
        "table": "players",
        "column": {
          "name": "rating",
          "type": "integer",
          "comment": "hello world",
          "check": {
            "name": "rating_check",
            "constraint": "rating > 0 AND rating < 100"
          },
          "nullable": false
        }
      }
    },
    {
      "create_index": {
        "name": "idx_player_rating",
        "table": "players",
        "columns": [
          "rating"
        ]
      }
    }
  ]
}
```

As of this PR the migration can be started.

The above migration does not validate yet, but it can be started
successfully with the `--skip-validation` flag to the `start` command.

Part of xataio#239
andrew-farries added a commit that referenced this issue Jan 20, 2025
…table` operations (#606)

Ensure that multi-operation migrations combining `alter_column` `SET NOT
NULL` and `rename_table` operations work as expected.

```json
{
  "name": "06_multi_operation",
  "operations": [
    {
      "rename_table": {
        "from": "items",
        "to": "products"
      }
    },
    {
      "alter_column": {
        "table": "products",
        "column": "name",
        "nullable": false,
        "up": "SELECT CASE WHEN name IS NULL THEN 'anonymous' ELSE name END",
        "down": "name || '_from_down_trigger'"
      }
    }
  ]
}
```

This migration renames a table and then sets a column's `nullability` on
the renamed table.

Previously the migration would fail as the `alter_column` operation was
unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 20, 2025
…column` operations (#607)

Ensure that multi-operation migrations combining `alter_column` `SET NOT
NULL` and `rename_column` operations work as expected.

```json
{
  "name": "06_multi_operation",
  "operations": [
    {
      "rename_column": {
        "table": "items",
        "from": "name",
        "to": "item_name"
      }
    },
    {
      "alter_column": {
        "table": "items",
        "column": "item_name",
        "nullable": false,
        "up": "SELECT CASE WHEN item_name IS NULL THEN 'anonymous' ELSE item_name END",
        "down": "item_name || '_from_down_trigger'"
      }
    }
  ]
}
```

This migration renames a column and then sets the renamed column's
`nullability`.

Previously the migration would fail as the `alter_column` operation was
unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 20, 2025
…table` operations (#608)

Ensure that multi-operation migrations combining `alter_column` `SET NOT
NULL` and `create_table` operations work as expected.

```json
{
  "name": "06_multi_operation",
  "operations": [
    {
      "create_table": {
        "name": "items",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "text",
            "nullable": true
          }
        ]
      }
    },
    {
      "alter_column": {
        "table": "items",
        "column": "name",
        "nullable": false,
        "up": "SELECT CASE WHEN name IS NULL THEN 'anonymous' ELSE name END",
        "down": "name || '_from_down_trigger'"
      }
    }
  ]
}
```

This migration creates a table and then sets a column's `nullability`.

Previously the migration would fail as the `alter_column` operation was
unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 20, 2025
…ion migrations (#609)

Ensure that multi-operation migrations combining `alter_column`
operations setting columns to `UNIQUE` works in combination with other
operations.

Add testcases for:

* rename table, set column unique
* rename table, rename column, set column unique
* create table, set column unique

Previously these migrations would fail as the `alter_column` operation
was unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 22, 2025
…tion migrations (#616)

Ensure that multi-operation migrations combining `alter_column`
operations setting column defaults work in combination with other
operations.

Add testcases for:

* rename table, set column default
* rename table, rename column, set default

Previously these migrations would fail as the `alter_column` operation
was unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 23, 2025
…eration migrations (#621)

Ensure that multi-operation migrations combining `alter_column`
operations setting foreign key constraints work in combination with
other operations.

Add testcases for:

* rename referencing table, set foreign key
* rename referencing table, rename referencing column, set foreign key
* rename referenced table, set foreign key,
* rename referenced column, set foreign key

Previously these migrations would fail as the `alter_column` operation
was unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 23, 2025
…on migrations (#622)

Ensure that multi-operation migrations combining `alter_column`
operations setting `CHECK` constraints work in combination with other
operations.

Add testcases for:

* rename table, set `CHECK` constraint
* rename table, rename column, set `CHECK` constraint

Previously these migrations would fail as the `alter_column` operation
was unaware of the changes made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Jan 27, 2025
…ration migrations (#626)

Ensure that multi-operation migrations combining `alter_column`
operations dropping `NOT NULL` constraints work in combination with
other operations.

Add testcases for:

* rename table, drop `NOT NULL` constraint
* rename table, rename column, drop `NOT NULL` constraint

The changes necessary to make these testcases pass were made as part of
earlier PRs for #239

Part of #239
@andrew-farries andrew-farries changed the title Support multiple operations acting on the same object in one migration Ensure that each operation type is aware of any preceding column or table renames Jan 31, 2025
@andrew-farries andrew-farries changed the title Ensure that each operation type is aware of any preceding column or table renames Ensure each operation type is aware of any preceding column or table renames Jan 31, 2025
andrew-farries added a commit that referenced this issue Feb 7, 2025
…on migrations (#661)

Ensure that multi-operation migrations combining `alter_column`
operations that change column type work in combination with other
operations.

Add testcases for:

* rename table, change column type
* rename table, rename column, change column type

Previously these migrations would fail as the `alter_column` operation
was unaware of the renames made by the preceding operation.

Part of #239
andrew-farries added a commit that referenced this issue Feb 7, 2025
…ns (#662)

Ensure that multi-operation migrations combining `create_index`
operations work in combination with other operations.

Add testcases for:

* rename table, create index
* rename table, rename column, create index

Part of #239
andrew-farries added a commit that referenced this issue Feb 11, 2025
…nt` operations (#674)

Add support for allowing the table and column on which a
`create_constraint` operation acts to be renamed by preceding operations
in the same migration. For example, ensure that a migration like this
works as expected:

```json
{
  "name": "16_multiple_ops",
  "operations": [
    {
      "rename_table": {
        "from": "items",
        "to": "products"
      }
    },
    {
      "rename_column": {
        "table": "products",
        "from": "name",
        "to": "item_name"
      }
    },
    {
      "create_constraint": {
        "table": "products",
        "type": "check",
        "name": "check_item_name",
        "columns": [ "item_name" ],
        "check": "length(item_name) > 3",
        "up": {
          "item_name": "item_name || '-from-up'"
        },
        "down": {
          "item_name": "item_name || '-from-down'"
        }
      }
    }
  ]
}
```

Here, a three operation migration first renames the `items` table to
`products`, renames the `name` field to `item_name` then adds a `CHECK`
constraint to the `item_name` field on the `products` table.

This PR covers the case of adding `CHECK` constraints only; further
(smaller) PRs may be needed for the other constraint types supported by
`create_constraint`.

Part of #239
andrew-farries added a commit that referenced this issue Feb 14, 2025
…nt` `FOREIGN KEY` operations (#682)

Ensure that `create_constraint` `FOREIGN KEY` operations can be preceded
by rename table and rename column operations as in the following
example:

```json
{
  "name": "22_multiple_ops",
  "operations": [
    {
      "rename_table": {
        "from": "items",
        "to": "products"
      }
    },
    {
      "rename_column": {
        "table": "products",
        "from": "owner",
        "to": "owner_id"
      }
    },
    {
      "create_constraint": {
        "table": "products",
        "type": "foreign_key",
        "name": "fk_items_users",
        "columns": ["owner_id"],
        "references": {
          "table": "users",
          "columns": ["id"]
        },
        "up": {
          "owner_id": "owner_id"
        },
        "down": {
          "owner_id": "owner_id"
        }
      }
    }
  ]
}
```

Follow up to #674 and #676. 

Part of #239.
andrew-farries added a commit that referenced this issue Feb 14, 2025
…n_constraint` operations (#684)

Ensure that `drop_multicolumn_constraint` operations can be preceded by
rename table and rename column operations as in the following example:

```json
{
  "name": "24_drop_constraint",
  "operations": [
    {
      "rename_table": {
        "from": "items",
        "to": "products"
      }
    },
    {
      "rename_column": {
        "table": "products",
        "from": "name",
        "to": "item_name"
      }
    },
    {
      "drop_multicolumn_constraint": {
        "table": "products",
        "name": "name_length",
        "up": {
          "item_name": "item_name"
        },
        "down": {
          "item_name": "SELECT CASE WHEN length(item_name) <= 3 THEN LPAD(item_name, 4, '-') ELSE item_name END"
        }
      }
    }
  ]
}
```

Part of #239.
@andrew-farries
Copy link
Collaborator Author

Closing this as complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants