SqlServer RevEng: Handle included index columns

[bricelam]

Fixes #17083

[ghost]

Hello @bricelam!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[smitpatel]

I was curious.
So when I use lambda based IncludeProperties, the annotation contains property names. If those properties are mapped to different column name, we handle it somewhere (couldn't find in source code yet).
In above we are storing column names rather than property names. Property names could be something different if column name is not valid C# identifier. Where & how that would be handled so that everything aligns properly?

[bricelam]

lol, probably a bug. See also #14087

[smitpatel]

Looks like changes from #14087 are moved. I verified in a console app that we do remapping just don't know where. If we have remapping mechanism then probably reveng also need to remap column names to generate property names when apply annotation to IModel.

[bricelam]

Hmm, I wonder where would that code would go. We don't know the property names here, and the model generator just blindly copies provider-specific annotations.

[AndriySvyryd]

efcore/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs

Line 151 in 10d9bb9

var includeColumns = (IReadOnlyList<string>)includeProperties

[bricelam]

That handles IIndex -> ITableIndex, but I don't think we have a provider hook for the inverse. I'll file an issue.

[AndriySvyryd]

We would need a provider-specific reveng annotation processor

[bricelam]

Ugg, adding tests that assert bugs is definitely an anti-pattern.