Fix migration for ints with default values

Ints with default values were mistakenly rendred as SERIAL.

Fixes #37

src/Npgsql.EntityFrameworkCore.PostgreSQL/Metadata/Internal/NpgsqlAnnotationNames.cs

@@ -26,7 +26,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal
     public static class NpgsqlAnnotationNames
     {
         public const string Prefix = "Npgsql:";
-        public const string Serial = "Serial";
+        public const string ValueGeneratedOnAdd = "ValueGeneratedOnAdd";
         public const string DefaultSequenceName = "DefaultSequenceName";
         public const string DefaultSequenceSchema = "DefaultSequenceSchema";
         public const string SequenceName = "SequenceName";

src/Npgsql.EntityFrameworkCore.PostgreSQL/Metadata/Internal/NpgsqlFullAnnotationNames.cs

@@ -28,7 +28,6 @@ public class NpgsqlFullAnnotationNames : RelationalFullAnnotationNames
         protected NpgsqlFullAnnotationNames(string prefix)
             : base(prefix)
         {
-            Serial = prefix + NpgsqlAnnotationNames.Serial;
             DefaultSequenceName = prefix + NpgsqlAnnotationNames.DefaultSequenceName;
             DefaultSequenceSchema = prefix + NpgsqlAnnotationNames.DefaultSequenceSchema;
             SequenceName = prefix + NpgsqlAnnotationNames.SequenceName;
@@ -40,7 +39,6 @@ protected NpgsqlFullAnnotationNames(string prefix)
 
         public new static NpgsqlFullAnnotationNames Instance { get; } = new NpgsqlFullAnnotationNames(NpgsqlAnnotationNames.Prefix);
 
-        public readonly string Serial;
         public readonly string DefaultSequenceName;
         public readonly string DefaultSequenceSchema;
         public readonly string SequenceName;

src/Npgsql.EntityFrameworkCore.PostgreSQL/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs

@@ -33,15 +33,12 @@ public class NpgsqlMigrationsAnnotationProvider : MigrationsAnnotationProvider
     {
         public override IEnumerable<IAnnotation> For(IProperty property)
         {
-            if (property.ValueGenerated == ValueGenerated.OnAdd &&
-                property.ClrType.IsIntegerForSerial()) {
-                yield return new Annotation(NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.Serial, true);
-            }
-
-            // TODO: Named sequences
-
-            // TODO: We don't support ValueGenerated.OnAddOrUpdate, so should we throw an exception?
-            // Other providers don't seem to...
+            // The migrations SQL generator gets the property's DefaultValue and DefaultValueSql.
+            // However, there's no way there to detect properties that have ValueGenerated.OnAdd
+            // *without* defining a default value; these should translate to SERIAL columns.
+            // So we add a custom annotation here to pass the information.
+            if (property.ValueGenerated == ValueGenerated.OnAdd)
+                yield return new Annotation(NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.ValueGeneratedOnAdd, true);
         }
 
         public override IEnumerable<IAnnotation> For(IIndex index)

src/Npgsql.EntityFrameworkCore.PostgreSQL/Migrations/NpgsqlMigrationsSqlGenerator.cs

@@ -91,8 +91,9 @@ protected override void Generate(AlterColumnOperation operation, IModel model, R
                     : TypeMapper.GetMapping(operation.ClrType).DefaultTypeName;
             }
 
-            var serial = operation.FindAnnotation(NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.Serial);
-            var isSerial = serial != null && (bool)serial.Value;
+            var generatedOnAddAnnotation = operation[NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.ValueGeneratedOnAdd];
+            var isSerial = generatedOnAddAnnotation != null && (bool)generatedOnAddAnnotation &&
+                operation.DefaultValue == null && operation.DefaultValueSql == null;
 
             var identifier = SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema);
             var alterBase = $"ALTER TABLE {identifier} ALTER COLUMN {SqlGenerationHelper.DelimitIdentifier(operation.Name)}";
@@ -142,7 +143,6 @@ protected override void Generate(AlterColumnOperation operation, IModel model, R
                         break;
                     default:
                         throw new NotImplementedException($"Not supporting creating IsIdentity for {type}");
-
                 }
             }
             else
@@ -382,18 +382,18 @@ protected override void Generate(DropIndexOperation operation, IModel model, Rel
         }
 
         protected override void ColumnDefinition(
-            string schema,
-            string table,
-            string name,
-            Type clrType,
-            string type,
+            [CanBeNull] string schema,
+            [CanBeNull] string table,
+            [NotNull] string name,
+            [NotNull] Type clrType,
+            [CanBeNull] string type,
             bool nullable,
-            object defaultValue,
-            string defaultValueSql,
-            string computedColumnSql,
-            IAnnotatable annotatable,
-            IModel model,
-            RelationalCommandListBuilder builder)
+            [CanBeNull] object defaultValue,
+            [CanBeNull] string defaultValueSql,
+            [CanBeNull] string computedColumnSql,
+            [NotNull] IAnnotatable annotatable,
+            [CanBeNull] IModel model,
+            [NotNull] RelationalCommandListBuilder builder)
         {
             Check.NotEmpty(name, nameof(name));
             Check.NotNull(annotatable, nameof(annotatable));
@@ -408,11 +408,13 @@ protected override void Generate(DropIndexOperation operation, IModel model, Rel
                     : TypeMapper.GetMapping(clrType).DefaultTypeName;
             }
 
-            // TODO: Maybe implement computed columns via functions?
-            // http://stackoverflow.com/questions/11165450/store-common-query-as-column/11166268#11166268
+            // An int-like property marked with OnAdd and without a default value is
+            // treated as a serial column
+            var generatedOnAddAnnotation = annotatable[NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.ValueGeneratedOnAdd];
+            var isSerial = generatedOnAddAnnotation != null && (bool)generatedOnAddAnnotation &&
+                defaultValue == null && defaultValueSql == null;
 
-            var serial = annotatable[NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.Serial];
-            if (serial != null && (bool)serial)
+            if (isSerial)
             {
                 switch (type)
                 {

src/Npgsql.EntityFrameworkCore.PostgreSQL/project.json

@@ -22,7 +22,7 @@
     "Microsoft.EntityFrameworkCore.Relational": "1.0.0-rc2-*",
     "Microsoft.Extensions.DependencyInjection.Abstractions": "1.0.0-rc2-*",
     "Microsoft.Extensions.DependencyInjection": "1.0.0-rc2-*",
-    "Npgsql": "3.1.0"
+    "Npgsql": "3.1.2"
   },
   "frameworks": {
     "net451": {},

test/Npgsql.EntityFrameworkCore.PostgreSQL.FunctionalTests/project.json

@@ -10,9 +10,10 @@
     "Microsoft.EntityFrameworkCore": "1.0.0-rc2-*",
     "Microsoft.EntityFrameworkCore.Relational": "1.0.0-rc2-*",
     "Microsoft.EntityFrameworkCore.Relational.Specification.Tests": "1.0.0-rc2-*",
-    "Npgsql.EntityFrameworkCore.PostgreSQL": "1.0.0-*",
     "Microsoft.Extensions.Configuration.Json": "1.0.0-*",
-    "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0-*"
+    "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0-*",
+    "Npgsql.EntityFrameworkCore.PostgreSQL": "1.0.0-*",
+    "Npgsql": "3.1.2"
   },
   "testRunner": "xunit",
   "frameworks": {

test/Npgsql.EntityFrameworkCore.PostgreSQL.Tests/Migrations/NpgsqlMigrationSqlGeneratorTest.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Text;
+using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Migrations;
@@ -330,6 +331,43 @@ public void CreatePostgresExtension()
                 Sql);
         }
 
+        [Fact]
+        public virtual void AddColumnOperation_serial()
+        {
+            Generate(new AddColumnOperation
+            {
+                Table = "People",
+                Name = "foo",
+                ClrType = typeof(int),
+                ColumnType = "int",
+                IsNullable = false,
+                [NpgsqlAnnotationNames.Prefix + NpgsqlAnnotationNames.ValueGeneratedOnAdd] = true
+            });
+
+            Assert.Equal(
+                "ALTER TABLE \"People\" ADD \"foo\" serial NOT NULL;" + EOL,
+                Sql);
+        }
+
+        [Fact]
+        public virtual void AddColumnOperation_with_int_defaultValue_isnt_serial()
+        {
+            Generate(
+                new AddColumnOperation
+                {
+                    Table = "People",
+                    Name = "foo",
+                    ClrType = typeof(int),
+                    ColumnType = "int",
+                    IsNullable = false,
+                    DefaultValue = "8"
+                });
+
+            Assert.Equal(
+                "ALTER TABLE \"People\" ADD \"foo\" int NOT NULL DEFAULT '8';" + EOL,
+                Sql);
+        }
+
         #endregion
     }
 }

test/Npgsql.EntityFrameworkCore.PostgreSQL.Tests/project.json

@@ -6,7 +6,7 @@
     "Microsoft.EntityFrameworkCore.Relational": "1.0.0-rc2-*",
     "Microsoft.EntityFrameworkCore.Relational.Specification.Tests": "1.0.0-rc2-*",
     "Npgsql.EntityFrameworkCore.PostgreSQL": "1.0.0-*",
-    "Npgsql": "3.1.0-*"
+    "Npgsql": "3.1.2"
   },
   "testRunner": "xunit",
   "frameworks": {