Add missing AggregateFaultCount column to V3_7 migration #120
Conversation
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request attempts to add the AggregateFaultCount column to the V3_7 database migration for the ActivityExecutionRecords table. However, this column was already added in the V3_5 migration (line 17 of V3_5.cs). The PR description incorrectly states that the column was omitted from the migration.
Changes:
- Adds
AggregateFaultCountcolumn to V3_7 Up() method (line 22) - this is a duplicate - Adds corresponding column removal to V3_7 Down() method (line 33) - this is incorrect
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Alter.Table("ActivityExecutionRecords").AddColumn("SchedulingActivityId").AsString().Nullable(); | ||
| Alter.Table("ActivityExecutionRecords").AddColumn("SchedulingWorkflowInstanceId").AsString().Nullable(); | ||
| Alter.Table("ActivityExecutionRecords").AddColumn("CallStackDepth").AsInt32().Nullable(); | ||
| Alter.Table("ActivityExecutionRecords").AddColumn("AggregateFaultCount").AsInt32().NotNullable().WithDefaultValue(0); |
There was a problem hiding this comment.
The AggregateFaultCount column was already added to the ActivityExecutionRecords table in the V3_5 migration (line 17 of V3_5.cs). Adding it again in V3_7 will cause migration failures because the column already exists. This line should be removed.
| Delete.Column("SchedulingActivityId").FromTable("ActivityExecutionRecords"); | ||
| Delete.Column("SchedulingWorkflowInstanceId").FromTable("ActivityExecutionRecords"); | ||
| Delete.Column("CallStackDepth").FromTable("ActivityExecutionRecords"); | ||
| Delete.Column("AggregateFaultCount").FromTable("ActivityExecutionRecords"); |
There was a problem hiding this comment.
This deletion should be removed because the AggregateFaultCount column was already added in the V3_5 migration, not V3_7. The V3_5 Down() method is responsible for removing this column.
The V3_7 database migration was incomplete - it added several new fields to
ActivityExecutionRecordsbut omitted theAggregateFaultCountcolumn despite it being present in the entity model and store mappings.Changes:
AggregateFaultCountcolumn (int, not null, default 0) to migrationUp()methodDown()method for rollback supportThe field was already implemented in:
ActivityExecutionRecordRecord.cs(line 88-89)DapperActivityExecutionRecordStore.csmapping logic (lines 146, 176, 200)This ensures the database schema matches the entity model when the migration runs.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.