-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
EF7 SqlServer Migration is trying to update columns on History table before creating the History table if any new columns are added in the same migration #29799
Labels
closed-fixed
The issue has been fixed and is/will be included in the release indicated by the issue milestone.
customer-reported
type-bug
Milestone
Comments
i'm able to repro this on latest bits. When processing AddColumnOperation we should check for the scenario where the current column is on temporal table but the old column is on regular table. |
Is there any update on this? We are still seeing this issue. |
@andersme We hope to be able to address this in EF8, but this may not happen due to other priorities. |
maumar
added a commit
that referenced
this issue
Nov 6, 2023
Before we used to put temporal annotations on temporal tables and all their columns, so that it's easier to process. Problem was that this would generate very noisy migrations when converting from regular table to temporal and vice versa. Every column would have an AlterColumn operation (which we would ignore during processing, but they were nonetheless generated in migration files). Also, we were using relatively simple logic to track state of our temporal tables. Some operations require temporary disabling of the versioning/period, and we need to keep track of that so that we don't try to disable period twice, or forget to enable it later. The way we did it could lead to invalid SQL in some non-trivial scenarios (e.g. converting table to temporal and adding a new column at the same time) The new approach is to only put temporal annotations on the table itself, and the period columns. Regular columns of the temporal table don't have any temporal annotations on them anymore and we reason about temporal information based on other table-based migration operation in the batch and, if need be, on the relational model. We also keep track of the actual temporal information for every operation (rather than keeping global dictionaries of period/version), so that complex migrations, involving multiple operations are more robust. To achieve that we compute the initial (temporal) state of all the tables involved in the migration. We scan all the table operations, and if some info is missing we get it from relational model. Then we do the proper processing of the migration operations - when we encounter table operation, we update the temporal information for that table (since table operations contain relevant temporal annotations). For all other operations we extract the current temporal state for the table involved, and reason based on that info. Fixes #27459 - SQL Server Migrations: Review temporal table annotations Fixes #29536 - EF Core IsTemporal() creates huge migration Fixes #29799 - EF7 SqlServer Migration is trying to update columns on History table before creating the History table if any new columns are added in the same migration
maumar
added a commit
that referenced
this issue
Nov 13, 2023
Before we used to put temporal annotations on temporal tables and all their columns, so that it's easier to process. Problem was that this would generate very noisy migrations when converting from regular table to temporal and vice versa. Every column would have an AlterColumn operation (which we would ignore during processing, but they were nonetheless generated in migration files). Also, we were using relatively simple logic to track state of our temporal tables. Some operations require temporary disabling of the versioning/period, and we need to keep track of that so that we don't try to disable period twice, or forget to enable it later. The way we did it could lead to invalid SQL in some non-trivial scenarios (e.g. converting table to temporal and adding a new column at the same time) The new approach is to only put temporal annotations on the table itself, and the period columns. Regular columns of the temporal table don't have any temporal annotations on them anymore and we reason about temporal information based on other table-based migration operation in the batch and, if need be, on the relational model. We also keep track of the actual temporal information for every operation (rather than keeping global dictionaries of period/version), so that complex migrations, involving multiple operations are more robust. To achieve that we compute the initial (temporal) state of all the tables involved in the migration. We scan all the table operations, and if some info is missing we get it from relational model. Then we do the proper processing of the migration operations - when we encounter table operation, we update the temporal information for that table (since table operations contain relevant temporal annotations). For all other operations we extract the current temporal state for the table involved, and reason based on that info. Fixes #27459 - SQL Server Migrations: Review temporal table annotations Fixes #29536 - EF Core IsTemporal() creates huge migration Fixes #29799 - EF7 SqlServer Migration is trying to update columns on History table before creating the History table if any new columns are added in the same migration
maumar
added a commit
that referenced
this issue
Nov 13, 2023
Before we used to put temporal annotations on temporal tables and all their columns, so that it's easier to process. Problem was that this would generate very noisy migrations when converting from regular table to temporal and vice versa. Every column would have an AlterColumn operation (which we would ignore during processing, but they were nonetheless generated in migration files). Also, we were using relatively simple logic to track state of our temporal tables. Some operations require temporary disabling of the versioning/period, and we need to keep track of that so that we don't try to disable period twice, or forget to enable it later. The way we did it could lead to invalid SQL in some non-trivial scenarios (e.g. converting table to temporal and adding a new column at the same time) The new approach is to only put temporal annotations on the table itself, and the period columns. Regular columns of the temporal table don't have any temporal annotations on them anymore and we reason about temporal information based on other table-based migration operation in the batch and, if need be, on the relational model. We also keep track of the actual temporal information for every operation (rather than keeping global dictionaries of period/version), so that complex migrations, involving multiple operations are more robust. To achieve that we compute the initial (temporal) state of all the tables involved in the migration. We scan all the table operations, and if some info is missing we get it from relational model. Then we do the proper processing of the migration operations - when we encounter table operation, we update the temporal information for that table (since table operations contain relevant temporal annotations). For all other operations we extract the current temporal state for the table involved, and reason based on that info. Fixes #27459 - SQL Server Migrations: Review temporal table annotations Fixes #29536 - EF Core IsTemporal() creates huge migration Fixes #29799 - EF7 SqlServer Migration is trying to update columns on History table before creating the History table if any new columns are added in the same migration
maumar
added a commit
that referenced
this issue
Nov 13, 2023
Before we used to put temporal annotations on temporal tables and all their columns, so that it's easier to process. Problem was that this would generate very noisy migrations when converting from regular table to temporal and vice versa. Every column would have an AlterColumn operation (which we would ignore during processing, but they were nonetheless generated in migration files). Also, we were using relatively simple logic to track state of our temporal tables. Some operations require temporary disabling of the versioning/period, and we need to keep track of that so that we don't try to disable period twice, or forget to enable it later. The way we did it could lead to invalid SQL in some non-trivial scenarios (e.g. converting table to temporal and adding a new column at the same time) The new approach is to only put temporal annotations on the table itself, and the period columns. Regular columns of the temporal table don't have any temporal annotations on them anymore and we reason about temporal information based on other table-based migration operation in the batch and, if need be, on the relational model. We also keep track of the actual temporal information for every operation (rather than keeping global dictionaries of period/version), so that complex migrations, involving multiple operations are more robust. To achieve that we compute the initial (temporal) state of all the tables involved in the migration. We scan all the table operations, and if some info is missing we get it from relational model. Then we do the proper processing of the migration operations - when we encounter table operation, we update the temporal information for that table (since table operations contain relevant temporal annotations). For all other operations we extract the current temporal state for the table involved, and reason based on that info. Fixes #27459 - SQL Server Migrations: Review temporal table annotations Fixes #29536 - EF Core IsTemporal() creates huge migration Fixes #29799 - EF7 SqlServer Migration is trying to update columns on History table before creating the History table if any new columns are added in the same migration
maumar
added
the
closed-fixed
The issue has been fixed and is/will be included in the release indicated by the issue milestone.
label
Nov 13, 2023
Experiencing the same issue with EF Core 8.0.4 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
closed-fixed
The issue has been fixed and is/will be included in the release indicated by the issue milestone.
customer-reported
type-bug
I ran into an issue when setting a table to include it's history counterpart, where it will attempt to update the columns on the history table, before the history table is created, if there are any additional columns also being added, like
delete_date
below.I couldn't find any know issues or roadmap items regarding Temporal/History tables and ordering of SQL queries during the
Update-Database
orScript-Migration
process. So I don't know if this is a known issue or not, and I know Temporal tables are still being worked on overall. But here are my findings if it helps.I am unable to upload sample project from my work machine, I might be able to reporduce it from personal machine later if needed.
Steps to reproduce:
.IsTemporal(true)
Update-Database
will fail on second migrationHere is the generated migration from the C# changes for step 5, the second migration.
This is the SQL generated when running
Script-Migration
targeting only the above migration change.Here is the output when attempting to run
Update-Database
as shown on the first line.EF Core version: 7.0.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0
Operating system: Win 10
IDE: Visual Studio 2022 17.4.1
The text was updated successfully, but these errors were encountered: