[release/5.0] SqlServer RevEng: Stop scaffolding included index columns #23083
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
More people are hitting dotnet#22150 than we anticipated. Give that this configuration isn't needed when mapping to an existing column, we've decided to stop scaffolding them entirely until we can fix that particular issue.
|
Good decision... |
smitpatel
approved these changes
Oct 23, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #22150
Description
In 5.0 we started to reverse-engineer "included columns" for indexes. However, we have a bug where if the column names are different from the mapped property names, then the result is incorrect and fails at runtime. Fixing this bug requires significant work and is not something we can do in a patch. Therefore, we want to pull this feature.
Customer Impact
We've had several people run into this on the previews/RCs, and the reports keep coming. On reflection, we think it's likely that many customers will have column names that don't match, and so will hit this. The workaround is to edit the generated code to fix it. But this is manual and could involve editing hundreds of lines of code. This would then need to be done again if the database is later re-reverse-engineered, which is common for some development flows. ("Database First").
On the flip side, the feature itself is not very important. Therefore, we want to pull the feature from 5.0. This will not break any existing applications since this is a design-time reverse-engineering feature. It will be a take-back of a small feature that has been in the previews and RCs.
How found
Multiple customer reports.
Test coverage
We are missing test coverage in this area.
Regression?
Yes, we believe many customers will hit this with 5.0 when reverse-engineering databases that worked in 3.1.
Risk
Very low. It's a very simple feature, and removal is very unlikely to have any knock-on effects. It is also a design-time feature, so there is no chance of breaking existing applications at runtime.