Skip to content

fix: don't detect changes if enum values were reordered #108

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

serozhenka
Copy link

Description

I've noticed that when changing the order of elements in the enum the alembic-postgresql-enum detects changes and tries to sync values, even though it shouldn't. Here is an example of the upgrade part of the migration when reordering values in ["active", "passive"] Enum to ["passive", "active"]:

# ### commands auto generated by Alembic - please adjust! ###
op.sync_enum_values(
    enum_schema='public',
    enum_name='user_status',
    new_values=['passive', 'active'],
    affected_columns=[TableReference(table_schema='public', table_name='users', column_name='status')],
    enum_values_to_rename=[],
)
# ### end Alembic commands ###

The fix was a simpler one-liner sorting the tuple elements before comparison.
Added a single test case to cover that issue.
Hope for positive feedback here!

Checklist

This pull request is:

  • [] A documentation / typographical / small typing error fix
    • Good to go, no issue or tests are needed
  • A short code fix
    • please include the issue number, and create an issue if none exists, which
      must include a complete example of the issue. one line code fixes without an
      issue and demonstration will not be accepted.
    • Please include: Fixes: #<issue number> in the commit message
    • please include tests. one line code fixes without tests will not be accepted.
  • [] A new feature implementation
    • please include the issue number, and create an issue if none exists, which must
      include a complete example of how the feature would look.
    • Please include: Fixes: #<issue number> in the commit message
    • please include tests.

Have a nice day!

@RustyGuard
Copy link
Member

Order of enum values actually matters in postgresql.

https://www.postgresql.org/docs/current/datatype-enum.html#DATATYPE-ENUM-ORDERING

So this behavior is a feature, not a bug

@serozhenka
Copy link
Author

I see it, you are right.
Do you feel like this should be closed or it can be made a non-default option in the config?

@RustyGuard
Copy link
Member

I think it should be an opt-out feature. Otherwise no one (even those who need it) will use it

@RustyGuard
Copy link
Member

Looks good to me, this pr will be a part of the next release

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

Successfully merging this pull request may close these issues.

2 participants