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

Fix Id Sequences on Constraint, Goals, and Conditions #1401

Merged
merged 2 commits into from
Apr 12, 2024

Conversation

Mythicaeda
Copy link
Contributor

@Mythicaeda Mythicaeda commented Apr 11, 2024

  • Tickets addressed: Hotfix
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

During the migrations that split constraints, goals, and conditions into metadata and definitions, we preserved the IDs. This meant, however, that the ID sequence used to generate new IDs were not advanced, causing issues with inserting new rows in these tables initially.

This PR creates a migration that advances the ID sequence on these tables to where it should be (the current max id + 1, or 1 if the table is empty). The down migrations are empty aside from marking the rollback as there is nothing to revert.

Verification

I first tested the migration against an empty database.

I then:

  1. Spun up a 2.4.0 Aerie instance (the last one before versioning constraints)
  2. Added 5 Constraints, Goals, and Conditions
  3. Migrated to this branch
  4. Created a new Constraint, Goal, and Condition

Documentation

No docs need to be updated.

@Mythicaeda Mythicaeda self-assigned this Apr 11, 2024
@Mythicaeda Mythicaeda requested a review from a team as a code owner April 11, 2024 22:12
@Mythicaeda Mythicaeda added this to the FY24 Q3 - Bug Fixes milestone Apr 11, 2024
@Mythicaeda Mythicaeda added the fix A bug fix label Apr 11, 2024
Copy link
Collaborator

@mattdailis mattdailis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Do we feel pretty good that we don't need to do this for other tables?

@Mythicaeda
Copy link
Contributor Author

Mythicaeda commented Apr 12, 2024

Yeah. I went and checked the rest of the repo and the only other time we ever used the phrase "generated by default" (including within migrations) is on activity_directive and merge_staging_area. I'm not thrilled about the usage in merge_staging_area and imagine that that was just a result of copying the activity_directive table definition and tweaking it, but felt fixing it was out of scope for this PR.

@Mythicaeda Mythicaeda merged commit c8fb740 into develop Apr 12, 2024
10 checks passed
@Mythicaeda Mythicaeda deleted the fix/update-sequences branch April 12, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A bug fix
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants