Migrations: Honor [Column(Order)] in CreateTable operation

Resolves dotnet#10059

src/EFCore.Design/Metadata/Internal/ScaffoldingAnnotationNames.cs

@@ -19,14 +19,6 @@ public static class ScaffoldingAnnotationNames
         /// </summary>
         public const string Prefix = "Scaffolding:";
 
-        /// <summary>
-        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-        ///     any release. You should only use it directly in your code with extreme caution and knowing that
-        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-        /// </summary>
-        public const string ColumnOrdinal = Prefix + "ColumnOrdinal";
-
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs

@@ -606,7 +606,7 @@ private void GenerateProperty(IProperty property)
                 .FilterIgnoredAnnotations(property.GetAnnotations())
                 .ToDictionary(a => a.Name, a => a);
             _annotationCodeGenerator.RemoveAnnotationsHandledByConventions(property, annotations);
-            annotations.Remove(ScaffoldingAnnotationNames.ColumnOrdinal);
+            annotations.Remove(RelationalAnnotationNames.ColumnOrder);
 
             if (_useDataAnnotations)
             {
@@ -885,7 +885,7 @@ private void GenerateManyToMany(ISkipNavigation skipNavigation)
                                     .FilterIgnoredAnnotations(property.GetAnnotations())
                                     .ToDictionary(a => a.Name, a => a);
                                 _annotationCodeGenerator.RemoveAnnotationsHandledByConventions(property, propertyAnnotations);
-                                propertyAnnotations.Remove(ScaffoldingAnnotationNames.ColumnOrdinal);
+                                propertyAnnotations.Remove(RelationalAnnotationNames.ColumnOrder);
 
                                 if ((!_useNullableReferenceTypes || property.ClrType.IsValueType)
                                     && !property.IsNullable

src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs

@@ -272,7 +272,7 @@ protected virtual void GenerateProperties(IEntityType entityType)
         {
             Check.NotNull(entityType, nameof(entityType));
 
-            foreach (var property in entityType.GetProperties().OrderBy(p => p.GetColumnOrdinal()))
+            foreach (var property in entityType.GetProperties().OrderBy(p => p.GetColumnOrder() ?? -1))
             {
                 GenerateComment(property.GetComment());
 

src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs

@@ -519,7 +519,7 @@ protected virtual EntityTypeBuilder VisitColumns(EntityTypeBuilder builder, ICol
                 property.IsConcurrencyToken();
             }
 
-            property.Metadata.SetColumnOrdinal(column.Table.Columns.IndexOf(column));
+            property.Metadata.SetColumnOrder(column.Table.Columns.IndexOf(column));
 
             property.Metadata.AddAnnotations(
                 column.GetAnnotations().Where(

src/EFCore.Relational/Design/AnnotationCodeGenerator.cs

@@ -55,6 +55,10 @@ private static readonly MethodInfo _propertyHasColumnNameMethodInfo
             = typeof(RelationalPropertyBuilderExtensions).GetRequiredRuntimeMethod(
                 nameof(RelationalPropertyBuilderExtensions.HasColumnName), typeof(PropertyBuilder), typeof(string));
 
+        private static readonly MethodInfo _propertyHasColumnOrderMethodInfo
+            = typeof(RelationalPropertyBuilderExtensions).GetRequiredRuntimeMethod(
+                nameof(RelationalPropertyBuilderExtensions.HasColumnOrder), typeof(PropertyBuilder), typeof(int?));
+
         private static readonly MethodInfo _propertyHasDefaultValueSqlMethodInfo1
             = typeof(RelationalPropertyBuilderExtensions).GetRequiredRuntimeMethod(
                 nameof(RelationalPropertyBuilderExtensions.HasDefaultValueSql), typeof(PropertyBuilder));
@@ -221,6 +225,10 @@ public virtual IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
                 annotations,
                 RelationalAnnotationNames.ColumnName, _propertyHasColumnNameMethodInfo, methodCallCodeFragments);
 
+            GenerateSimpleFluentApiCall(
+                annotations,
+                RelationalAnnotationNames.ColumnOrder, _propertyHasColumnOrderMethodInfo, methodCallCodeFragments);
+
             if (TryGetAndRemove(annotations, RelationalAnnotationNames.DefaultValueSql, out string? defaultValueSql))
             {
                 methodCallCodeFragments.Add(

src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs

@@ -367,6 +367,7 @@ public override void Generate(IProperty property, CSharpRuntimeAnnotationCodeGen
             }
             else
             {
+                annotations.Remove(RelationalAnnotationNames.ColumnOrder);
                 annotations.Remove(RelationalAnnotationNames.Comment);
                 annotations.Remove(RelationalAnnotationNames.Collation);
 

src/EFCore.Relational/Diagnostics/MigrationColumnOperationEventData.cs

@@ -0,0 +1,34 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Diagnostics;
+using Microsoft.EntityFrameworkCore.Migrations.Operations;
+
+namespace Microsoft.EntityFrameworkCore.Diagnostics
+{
+    /// <summary>
+    ///     The <see cref="DiagnosticSource"/> event payload for events that reference a Migrations column operation.
+    /// </summary>
+    public class MigrationColumnOperationEventData : EventData
+    {
+        /// <summary>
+        ///     Initializes a new instance of the <see cref="MigrationColumnOperationEventData"/> class.
+        /// </summary>
+        /// <param name="eventDefinition"> The event definition. </param>
+        /// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
+        /// <param name="columnOperation"> The column operation. </param>
+        public MigrationColumnOperationEventData(
+            EventDefinitionBase eventDefinition,
+            Func<EventDefinitionBase, EventData, string> messageGenerator,
+            ColumnOperation columnOperation)
+            : base(eventDefinition, messageGenerator)
+            => ColumnOperation = columnOperation;
+
+        /// <summary>
+        ///     Gets the column operation.
+        /// </summary>
+        /// <value> The column operation. </value>
+        public ColumnOperation ColumnOperation { get; }
+    }
+}

src/EFCore.Relational/Diagnostics/RelationalEventId.cs

@@ -72,6 +72,7 @@ private enum Id
             MigrationsNotApplied,
             MigrationsNotFound,
             MigrationAttributeMissingWarning,
+            ColumnOrderIgnoredWarning,
 
             // Query events
             QueryClientEvaluationWarning = CoreEventId.RelationalBaseId + 500,
@@ -88,6 +89,7 @@ private enum Id
             IndexPropertiesMappedToNonOverlappingTables,
             ForeignKeyPropertiesMappedToUnrelatedTables,
             OptionalDependentWithoutIdentifyingPropertyWarning,
+            DuplicateColumnOrders,
 
             // Update events
             BatchReadyForExecution = CoreEventId.RelationalBaseId + 700,
@@ -598,6 +600,19 @@ private static EventId MakeMigrationsId(Id id)
         /// </summary>
         public static readonly EventId MigrationAttributeMissingWarning = MakeMigrationsId(Id.MigrationAttributeMissingWarning);
 
+        /// <summary>
+        ///     <para>
+        ///         Column order was ignored.
+        ///     </para>
+        ///     <para>
+        ///         This event is in the <see cref="DbLoggerCategory.Migrations" /> category.
+        ///     </para>
+        ///     <para>
+        ///         This event uses the <see cref="MigrationColumnOperationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+        ///     </para>
+        /// </summary>
+        public static readonly EventId ColumnOrderIgnoredWarning = MakeMigrationsId(Id.ColumnOrderIgnoredWarning);
+
         private static readonly string _queryPrefix = DbLoggerCategory.Query.Name + ".";
 
         private static EventId MakeQueryId(Id id)
@@ -739,6 +754,19 @@ private static EventId MakeValidationId(Id id)
         public static readonly EventId OptionalDependentWithoutIdentifyingPropertyWarning
             = MakeValidationId(Id.OptionalDependentWithoutIdentifyingPropertyWarning);
 
+        /// <summary>
+        ///     <para>
+        ///         The configured column orders for a table contains duplicates.
+        ///     </para>
+        ///     <para>
+        ///         This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
+        ///     </para>
+        ///     <para>
+        ///         This event uses the <see cref="StoreObjectEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+        ///     </para>
+        /// </summary>
+        public static readonly EventId DuplicateColumnOrders = MakeValidationId(Id.DuplicateColumnOrders);
+
         private static readonly string _updatePrefix = DbLoggerCategory.Update.Name + ".";
 
         private static EventId MakeUpdateId(Id id)

src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs

@@ -18,6 +18,7 @@
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Migrations;
 using Microsoft.EntityFrameworkCore.Migrations.Internal;
+using Microsoft.EntityFrameworkCore.Migrations.Operations;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Storage.Internal;
@@ -3033,5 +3034,73 @@ private static string OptionalDependentWithAllNullPropertiesWarningSensitive(Eve
             var p = (UpdateEntryEventData)payload;
             return d.GenerateMessage(p.EntityEntry.EntityType.DisplayName(), p.EntityEntry.BuildCurrentValuesString(p.EntityEntry.EntityType.FindPrimaryKey()!.Properties));
         }
+
+        /// <summary>
+        ///     Logs the <see cref="RelationalEventId.DuplicateColumnOrders" /> event.
+        /// </summary>
+        /// <param name="diagnostics"> The diagnostics logger to use. </param>
+        /// <param name="storeObject"> The table. </param>
+        public static void DuplicateColumnOrders(this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics, StoreObjectIdentifier storeObject)
+        {
+            var definition = RelationalResources.LogDuplicateColumnOrders(diagnostics);
+
+            if (diagnostics.ShouldLog(definition))
+            {
+                definition.Log(diagnostics, storeObject.DisplayName());
+            }
+
+            if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+            {
+                var eventData = new StoreObjectEventData(
+                    definition,
+                    DuplicateColumnOrders,
+                    storeObject);
+
+                diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+            }
+        }
+
+        private static string DuplicateColumnOrders(EventDefinitionBase definition, EventData payload)
+        {
+            var d = (EventDefinition<string>)definition;
+            var p = (StoreObjectEventData)payload;
+
+            return d.GenerateMessage(p.StoreObject.DisplayName());
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public static void ColumnOrderIgnoredWarning(
+            this IDiagnosticsLogger<DbLoggerCategory.Migrations> diagnostics,
+            ColumnOperation operation)
+        {
+            var definition = RelationalResources.LogColumnOrderIgnoredWarning(diagnostics);
+
+            if (diagnostics.ShouldLog(definition))
+            {
+                definition.Log(diagnostics, (operation.Table, operation.Schema).FormatTable() + "." + operation.Name);
+            }
+
+            if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+            {
+                var eventData = new MigrationColumnOperationEventData(
+                    definition,
+                    ColumnOrderIgnoredWarning,
+                    operation);
+
+                diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+            }
+        }
+
+        private static string ColumnOrderIgnoredWarning(EventDefinitionBase definition, EventData payload)
+        {
+            var d = (EventDefinition<string>)definition;
+            var p = (MigrationColumnOperationEventData)payload;
+            return d.GenerateMessage((p.ColumnOperation.Table, p.ColumnOperation.Schema).FormatTable() + "." + p.ColumnOperation.Name);
+        }
     }
 }

src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs

@@ -530,5 +530,23 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
         /// </summary>
         [EntityFrameworkInternal]
         public EventDefinitionBase? LogOptionalDependentWithAllNullPropertiesSensitive;
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        [EntityFrameworkInternal]
+        public EventDefinitionBase? LogDuplicateColumnOrders;
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        [EntityFrameworkInternal]
+        public EventDefinitionBase? LogColumnOrderIgnoredWarning;
     }
 }

src/EFCore.Relational/Diagnostics/StoreObjectEventData.cs

@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Diagnostics;
+using Microsoft.EntityFrameworkCore.Metadata;
+
+namespace Microsoft.EntityFrameworkCore.Diagnostics
+{
+    /// <summary>
+    ///     A <see cref="DiagnosticSource" /> event payload class for events that have a table.
+    /// </summary>
+    public class StoreObjectEventData : EventData
+    {
+        /// <summary>
+        ///     Constructs the event payload.
+        /// </summary>
+        /// <param name="eventDefinition"> The event definition. </param>
+        /// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
+        /// <param name="storeObject"> The table. </param>
+        public StoreObjectEventData(
+            EventDefinitionBase eventDefinition,
+            Func<EventDefinitionBase, EventData, string> messageGenerator,
+            StoreObjectIdentifier storeObject)
+            : base(eventDefinition, messageGenerator)
+            => StoreObject = storeObject;
+
+        /// <summary>
+        ///     The table.
+        /// </summary>
+        public virtual StoreObjectIdentifier StoreObject { get; }
+    }
+}