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

CHECK constraints not dropped if one migrates from Boolean or Enum to another type #652

Open
zzzeek opened this issue Feb 4, 2020 · 1 comment
Labels
autogenerate for enums a long term subject, tagging issues related to this bug Something isn't working op directives

Comments

@zzzeek
Copy link
Member

zzzeek commented Feb 4, 2020

first described in #605. #605 will address the detection of boolean -> TINYINT which is the biggest problem. this issue will track if we decide we will try to DROP the CHECK constraints generated for boolean and enum. this drop could occur either as part of the operation itself, or could occur as autogenerate adding an explciit "drop_constraint()" op to the migration script. however, it becomes very complicated because without a constraint naming convention in place, there's no way to really automate this without doing some kind of search of all the constraints to find one that involves the target column and looks like the constraint that we added (or any check that involves the column alone and no others). That in itself would need to be at op time, not migration time, because again we wouldn't have a deterministic name of the constraint.

overall it seems a little like a "just put a note / warning and needs manual intervention" thing.

maybe Alembic shouldn't allow you to create such a type without the constraint having an explicit name. again not sure.

@zzzeek zzzeek added bug Something isn't working op directives labels Feb 4, 2020
@zzzeek zzzeek added the autogenerate for enums a long term subject, tagging issues related to this label Feb 5, 2020
@jvanasco
Copy link
Member

I cross referenced #699 to this ticket. #699 is essentially "renaming a Bool column causes a fatal error in MySql because of the autogenerated constraint ensuring the "bool" is an INT with 0 or 1."

openstack-mirroring pushed a commit to openstack/nova that referenced this issue Feb 6, 2021
Compact Liberty database migrations into a single migration,
'302_liberty.py'.

Users will now need to update to Liberty before updating to Mitaka or
later.

Specific changes include:

- Drop 'volumes', 'iscsi_targets' tables
- Add 'migration_type', 'hidden' columns to 'migrations' table
- Add 'last_seen_up' column to 'services' table
- Add index for 'uuid' column of 'virtual_interfaces' table
- Add 'forced_down' column to 'services' table
- Add 'version' column to 'services' table
- Add 'migration_context' column to 'migration_context' table
- Add index for 'instance_uuid' column of 'instance_system_metadata'
  table for PostgreSQL and SQLite; this was already present for MySQL

This hits the same issue seen previously of a constraint being added to
a boolean field in a shadow table on SQLite, despite this being disabled
for the main table. As before, we can ignore this given SQLite is not a
production DB.

We also hit another case of doing something to a main table but not its
shadow table, which will have to be resolved later.

Now that we can rely on flavor records having been migrated, we can
remove the 'db migrate_flavor_data' nova-manage command. This will be
done separately.

When testing, the previous base version was 279. It is now 301.

[1] sqlalchemy/alembic#652

Change-Id: I9933a9e9087868f1cd92100b3e82c35fe02cab09
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Feb 6, 2021
* Update nova from branch 'master'
  to 34b351b8b8d2a6d7d249763a858ff3c35c25f5d3
  - Merge "db: Compact Liberty database migrations"
  - db: Compact Liberty database migrations
    
    Compact Liberty database migrations into a single migration,
    '302_liberty.py'.
    
    Users will now need to update to Liberty before updating to Mitaka or
    later.
    
    Specific changes include:
    
    - Drop 'volumes', 'iscsi_targets' tables
    - Add 'migration_type', 'hidden' columns to 'migrations' table
    - Add 'last_seen_up' column to 'services' table
    - Add index for 'uuid' column of 'virtual_interfaces' table
    - Add 'forced_down' column to 'services' table
    - Add 'version' column to 'services' table
    - Add 'migration_context' column to 'migration_context' table
    - Add index for 'instance_uuid' column of 'instance_system_metadata'
      table for PostgreSQL and SQLite; this was already present for MySQL
    
    This hits the same issue seen previously of a constraint being added to
    a boolean field in a shadow table on SQLite, despite this being disabled
    for the main table. As before, we can ignore this given SQLite is not a
    production DB.
    
    We also hit another case of doing something to a main table but not its
    shadow table, which will have to be resolved later.
    
    Now that we can rely on flavor records having been migrated, we can
    remove the 'db migrate_flavor_data' nova-manage command. This will be
    done separately.
    
    When testing, the previous base version was 279. It is now 301.
    
    [1] sqlalchemy/alembic#652
    
    Change-Id: I9933a9e9087868f1cd92100b3e82c35fe02cab09
    Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autogenerate for enums a long term subject, tagging issues related to this bug Something isn't working op directives
Projects
None yet
Development

No branches or pull requests

2 participants