Fix Scheduling Spec Goals Delete (for develop)

[Mythicaeda]

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

Description

Sister PR of #1435.

PR Description from 1435:

These PRs fix two bugs with our scheduling specifications: 1) The migration for Versioning Scheduling Goals did not update the `when` condition of the `on update` trigger for scheduling specifications, leading to the stack depth unexpectedly exploding when trying to update a spec 38+ goals (the threshold may be lower, but I did not look into this). 2) The function used for `on delete` did not properly handle bulk deletes (as in a delete statement that deletes multiple rows). This function and its trigger have been updated to operate "per-statement" and go through all the rows deleted in reverse priority order. This ensures that

Independently of these bugs, I also put a depth check on the increment spec update when the goals subtable is updated trigger, as there's no reason for us to update the revision when a trigger changes a goal's priority.

A simple example of the second bug follows:
Let's say we have the following specification. Specification are being expressed as an array ordered by priority and "tailing" priorities will be expressed as null in order to avoid confusion caused by the array changing size mid-walkthrough.
[ Goal 1, Goal 2, Goal 3, Goal 4, Goal 5, Goal 6 ]

Now let us delete Goal 1, Goal 2, and Goal 5. The expected output for this action is[Goal 3, Goal 4, Goal 6, null, null, null]

Under the old logic, the following would happen:

1. Goal 1, Goal 2, and Goal 5 are deleted: [null, null, Goal 3, Goal 4, null, Goal 6]
2. The after delete trigger for Goal 1 is processed. As Goal 3, Goal 4, and Goal 6 have a strictly higher priority than Goal 1, their priorities are reduced by 1: [null, Goal 3, Goal 4, null, Goal 6, null]
3. The after delete trigger for Goal 2 is processed. As Goal 4 and Goal 6 have a strictly higher priority than Goal 2, their priorities are reduced by 1: [null, Goal 3 and Goal 4, null, Goal 6, null, null]
4. The after delete trigger for Goal 5 is processed. As no Goals have a strictly higher priority than Goal 5, no rows are updated: [null, Goal 3 and Goal 4, null, Goal 6, null, null]
5. An exception is raised, as Goal 3 and Goal 4 cannot share a priority.

Note that preserving the old per-row function but instead reducing goals with a priority greater than or equal to the current goal's would not have been sufficient, as it would lead to gaps in priorities (as well as conflicting priorities in different setups). For example, in the above example, Goal 6 has been placed at priority 4 before we process Goal 5's deletion, meaning it will not be moved to priority 3, even though that's where we would expect it to be at the end of the transaction, leaving a gap at priority 3.

Under the new logic, the following occurs:

1. Goal 1, Goal 2, and Goal 5 are deleted: [null, null, Goal 3, Goal 4, null, Goal 6]
2. The after delete for step 1 begins. Goal 5, as the deleted Goal with the highest priority, is processed first: [null, null, Goal 3, Goal 4, Goal 6, null]
3. Goal 2, as the deleted Goal with the second-highest priority, is processed next: [null, Goal 3, Goal 4, Goal 6, null, null]
4. Goal 1, as the deleted Goal with the lowest priority, is processed last: [Goal 3, Goal 4, Goal 6, null, null, null]
5. The after delete for step 1 completes.

The output in this case is exactly as expected, as by processing in reverse priority order, we guarantee that all goals with a strictly higher priority than that of the deleted goal we're processing is still at a strictly higher priority than that deleted goal, even if that priority has changed.

Verification

DB Tests were added to test bulk deleting from the both types of specs in three different scenarios: a chunk out of the middle, only the middle left, and many alternating rows. Against develop, these tests are expected to fail.

Documentation

As said in 1435, we need to document that PGCMP does not catch when conditions on triggers so that viewers are aware to keep their eye out for it.

Future work

As said in 1435, looking into extending PGCMP to implement the when condition trigger match.

[skovati]

Tests look good to me!