Add support for index sort order (descending)

[roji]

• Added a new IsDescending Fluent API and Data Annotation.
• It's implemented as a property on Index and not as an annotation, since it's a core concept. One downside is that it has to be on RuntimeIndex, though it's not a runtime feature (I made it always return Array.Empty).
• I simplified the implementation: if IsDescending is specified, it must explicitly specify all values for all properties.

/cc @AndriySvyryd would appreciate a good review as this touches upon lots of metadata/model stuff, and is non-annotation array metadata property.

Closes #4150

[AndriySvyryd]

I simplified the implementation: if IsDescending is specified, it must explicitly specify all values for all properties.

Are all indexable columns orderable? Couldn't there be some type for which specifying the order could be detrimental, like an encrypted column, for example?

[AndriySvyryd]

It's implemented as a property on Index and not as an annotation, since it's a core concept. One downside is that it has to be on RuntimeIndex, though it's not a runtime feature (I made it always return Array.Empty).

Throw RuntimeModelMissingData instead

[roji]

I simplified the implementation: if IsDescending is specified, it must explicitly specify all values for all properties.

Are all indexable columns orderable? Couldn't there be some type for which specifying the order could be detrimental, like an encrypted column, for example?

That's an interesting question, although note that in the migration SQL we still generate nothing (just as before this feature) unless you use the new API to specify "descending". In other words, the PR forces the user to specify true/false for all columns, but at the migration level we generate only DESC, never ASC (which is the universal default). So if there's some issue with indexes over encrypted columns, that issue already exists today.

Throw RuntimeModelMissingData instead

Yeah, I tried that but RelationalModel accesses that IIndex.IsDescending property when building TableIndex - that seems to be a difference between having it as a real property vs. annotation.

[AndriySvyryd]

Yeah, I tried that but RelationalModel accesses that IIndex.IsDescending property when building TableIndex - that seems to be a difference between having it as a real property vs. annotation.

Calculate TableIndex.IsDescending lazily using ITableIndex.MappedIndexes.

[AndriySvyryd]

And don't forget to update RelationalIndexExtensions.AreCompatible

[AndriySvyryd]

Keep indexBuilder updated; Fluent API doesn't guarantee that old builders remain valid.

[roji]

I was wondering about that - saw some other places where it wasn't updated. May be worth doing a pass to check.

[AndriySvyryd]

Add a test to ConventionDispatcherTest

[AndriySvyryd]

You can call ((IReadOnlyIndex)this).DisplayName() instead of overriding

[roji]

The problem is that recurses into the same call: the public DisplayName implements the interface on IReadOnlyIndex...

Compare EntityType.DisplayName which happens to be private - there it's safe to do it (and I get an IDE warning with Non-public method 'DisplayName' hides method with default implementation in interface IReadOnlyTypeBase). But with Index we want to call DisplayName() from EntityType.cs, so it needs to be public...

We could probably work around this with something fancy, but is it worth it...

[AndriySvyryd]

The problem is that recurses into the same call: the public DisplayName implements the interface on IReadOnlyIndex...

That's why I said "instead of overriding", i.e. don't have this method on Index at all.

Compare EntityType.DisplayName which happens to be private

It's private by design 😃

[roji]

Ah I see... So this would have to be done here, but also in EntityType.cs where we call this twice... I dunno, overriding seems a bit nicer even if it duplicates the logic - it's very unlikely to change etc.