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

Procedural scheduling migration fix patch #1601

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

skovati
Copy link
Contributor

@skovati skovati commented Nov 12, 2024

Fixes an issue discovered by @parkerabercrombie during database migration while upgrading from Aerie 2.11.2 to 3.1.0:

  • While on 2.11.2, there was a plan that had some scheduling goals which were run, and produced scheduling goal analysis rows in the database.
  • Some of these goals were removed from the plan after the scheduling run, which caused the scheduling analysis rows to refer to a goal_id which was no longer in the plan spec (didn't cause any issues yet).
  • The up.sql migration 10 assigns goal_invocation_ids to all goal invocations in the plan spec, and propagates these IDs to the corresponding entries in the goal analysis tables.
  • This process expected that all analysis rows would refer to goals that still existed in the plan spec, but since some goals were removed from the spec, the associated analysis rows were assigned null as their goal_invocation_id, which is not legal to be used as part of a primary key
  • This caused the migration to fail when trying to set the primary key to include this column

This PR fixes the issue by assigning a "dummy value" to any goal analysis rows which would have suffered from this problem - instead of setting the goal_invocation_id to null, we (somewhat arbitrarily) set it to the negative of the goal id which instantiated it. This could have been any valid integer, but we decided to use this as the value since it gives us a bit more information about where the analysis came from.

(summarized by @dandelany )

@skovati skovati changed the title v3.1.1 migration fix patch Procedural scheduling migration fix patch Nov 12, 2024
Copy link
Collaborator

@dandelany dandelany left a comment

Choose a reason for hiding this comment

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

discussed with @skovati and reviewed with @mattdailis

@dandelany dandelany marked this pull request as ready for review November 12, 2024 22:31
@dandelany dandelany requested a review from a team as a code owner November 12, 2024 22:31
@dandelany dandelany merged commit 84d78d3 into develop Nov 12, 2024
14 checks passed
@dandelany dandelany deleted the fix/proc-sched-migration-error branch November 12, 2024 22:32
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.

3 participants