Merge branch 'hotfix/3.1.2'

.github/workflows/build.yml

@@ -87,11 +87,11 @@ jobs:
 
       - name: Pack NuGet packages (CI versions)
         if: startsWith(github.ref, 'refs/heads/')
-        run: dotnet pack --configuration Release --output nupkgs --version-suffix "ci.$(date -u +%Y%m%dT%H%M%S)+sha.${GITHUB_SHA:0:9}"
+        run: dotnet pack --configuration Release --output nupkgs --version-suffix "ci.$(date -u +%Y%m%dT%H%M%S)+sha.${GITHUB_SHA:0:9}" -p:ContinuousIntegrationBuild=true
 
       - name: Pack NuGet packages (Release versions)
         if: startsWith(github.ref, 'refs/tags/v')
-        run: dotnet pack --configuration Release --output nupkgs
+        run: dotnet pack --configuration Release --output nupkgs -p:ContinuousIntegrationBuild=true
 
       - name: Upload artifacts
         uses: actions/upload-artifact@v1

Directory.Build.props

@@ -1,15 +1,15 @@
 <Project>
   <!-- Package configuration -->
   <PropertyGroup>
-    <Copyright>Copyright 2019 © The Npgsql Development Team</Copyright>
+    <Copyright>Copyright 2020 © The Npgsql Development Team</Copyright>
     <Company>Npgsql</Company>
-    <VersionPrefix>3.1.1.2</VersionPrefix>
+    <VersionPrefix>3.1.2</VersionPrefix>
 
     <RepositoryType>git</RepositoryType>
     <RepositoryUrl>git://github.com/npgsql/efcore.pg</RepositoryUrl>
 
     <TargetFrameworks>netstandard2.0</TargetFrameworks>
-
+    <PublishRepositoryUrl>true</PublishRepositoryUrl>
     <PackageLicenseExpression>PostgreSQL</PackageLicenseExpression>
     <PackageProjectUrl>https://github.com/npgsql/efcore.pg</PackageProjectUrl>
     <PackageIcon>postgresql.png</PackageIcon>

Directory.Build.targets

@@ -1,19 +1,20 @@
 <Project>
   <ItemGroup>
-    <PackageReference Update="Microsoft.EntityFrameworkCore" Version="3.1.1" />
-    <PackageReference Update="Microsoft.EntityFrameworkCore.Relational" Version="3.1.1" />
-    <PackageReference Update="Microsoft.EntityFrameworkCore.Abstractions" Version="3.1.1" />
-    <PackageReference Update="Microsoft.EntityFrameworkCore.Relational.Specification.Tests" Version="3.1.1" />
-    <PackageReference Update="Microsoft.EntityFrameworkCore.Design" Version="3.1.1" />
+    <PackageReference Update="Microsoft.EntityFrameworkCore" Version="3.1.2" />
+    <PackageReference Update="Microsoft.EntityFrameworkCore.Relational" Version="3.1.2" />
+    <PackageReference Update="Microsoft.EntityFrameworkCore.Abstractions" Version="3.1.2" />
+    <PackageReference Update="Microsoft.EntityFrameworkCore.Relational.Specification.Tests" Version="3.1.2" />
+    <PackageReference Update="Microsoft.EntityFrameworkCore.Design" Version="3.1.2" />
 
-    <PackageReference Update="Microsoft.Extensions.Configuration.Json" Version="3.1.1" />
-    <PackageReference Update="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="3.1.1" />
-    <PackageReference Update="Microsoft.Extensions.Logging" Version="3.1.1" />
+    <PackageReference Update="Microsoft.Extensions.Configuration.Json" Version="3.1.2" />
+    <PackageReference Update="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="3.1.2" />
+    <PackageReference Update="Microsoft.Extensions.Logging" Version="3.1.2" />
 
     <PackageReference Update="Npgsql" Version="4.1.3" />
     <PackageReference Update="Npgsql.NodaTime" Version="4.1.3" />
     <PackageReference Update="Npgsql.NetTopologySuite" Version="4.1.3" />
 
+    <PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0" />
     <PackageReference Update="Microsoft.NET.Test.Sdk" Version="16.4.0" />
     <PackageReference Update="xunit" Version="2.4.1" />
     <PackageReference Update="xunit.runner.visualstudio" Version="2.4.1" />

src/Directory.Build.props

@@ -7,6 +7,6 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="SourceLink.Create.CommandLine" Version="2.8.3" PrivateAssets="All" /> 
+    <PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" />
   </ItemGroup>
 </Project>

src/EFCore.PG.NTS/Storage/Internal/NpgsqlNetTopologySuiteTypeMappingSourcePlugin.cs

@@ -15,16 +15,23 @@ public class NpgsqlNetTopologySuiteTypeMappingSourcePlugin : IRelationalTypeMapp
         // rather late by SingletonOptionsInitializer
         readonly INpgsqlNetTopologySuiteOptions _options;
 
-        static readonly Dictionary<string, Type> SubTypeNameToClrType = new Dictionary<string, Type>(StringComparer.OrdinalIgnoreCase)
+        static bool TryGetClrType(string subtypeName, out Type clrType)
         {
-            { "POINT",              typeof(Point) },
-            { "LINESTRING",         typeof(LineString) },
-            { "POLYGON",            typeof(Polygon) },
-            { "MULTIPOINT",         typeof(MultiPoint) },
-            { "MULTILINESTRING",    typeof(MultiLineString) },
-            { "MULTIPOLYGON",       typeof(MultiPolygon) },
-            { "GEOMETRYCOLLECTION", typeof(GeometryCollection) }
-        };
+            clrType = subtypeName switch
+            {
+                "POINT"              => typeof(Point),
+                "LINESTRING"         => typeof(LineString),
+                "POLYGON"            => typeof(Polygon),
+                "MULTIPOINT"         => typeof(MultiPoint),
+                "MULTILINESTRING"    => typeof(MultiLineString),
+                "MULTIPOLYGON"       => typeof(MultiPolygon),
+                "GEOMETRYCOLLECTION" => typeof(GeometryCollection),
+                "GEOMETRY"           => typeof(Geometry),
+                _                    => null
+            };
+
+            return clrType != null;
+        }
 
         public NpgsqlNetTopologySuiteTypeMappingSourcePlugin([NotNull] INpgsqlNetTopologySuiteOptions options)
             => _options = Check.NotNull(options, nameof(options));
@@ -41,7 +48,7 @@ public virtual RelationalTypeMapping FindMapping(in RelationalTypeMappingInfo ma
 
             if (storeTypeName != null)
             {
-                if (!TryParseStoreTypeName(storeTypeName, out isGeography, out var parsedSubtype, out var _))
+                if (!TryParseStoreTypeName(storeTypeName, out _, out isGeography, out var parsedSubtype, out _, out _))
                     return null;
                 if (clrType == null)
                     clrType = parsedSubtype;
@@ -56,22 +63,31 @@ public virtual RelationalTypeMapping FindMapping(in RelationalTypeMappingInfo ma
         }
 
         /// <summary>
-        /// Given a PostGIS store type name (e.g. GEOMETRY, GEOGRAPHY(Point, 4326)), attempts to parse it and return its components.
+        /// Given a PostGIS store type name (e.g. GEOMETRY, GEOGRAPHY(Point, 4326), GEOMETRY(LineStringM, 4326)),
+        /// attempts to parse it and return its components.
         /// </summary>
-        public static bool TryParseStoreTypeName(string storeTypeName, out bool isGeography, out Type clrType, out int srid)
+        public static bool TryParseStoreTypeName(
+            string storeTypeName,
+            out string subtypeName,
+            out bool isGeography,
+            out Type clrType,
+            out int srid,
+            out Ordinates ordinates)
         {
             storeTypeName = storeTypeName.Trim();
+            subtypeName = storeTypeName;
             isGeography = false;
             clrType = null;
             srid = -1;
+            ordinates = Ordinates.AllOrdinates;
 
             var openParen = storeTypeName.IndexOf("(", StringComparison.Ordinal);
 
             var baseType = openParen > 0 ? storeTypeName.Substring(0, openParen).Trim() : storeTypeName;
 
-            if (baseType.Equals("geometry", StringComparison.OrdinalIgnoreCase))
+            if (baseType.Equals("GEOMETRY", StringComparison.OrdinalIgnoreCase))
                 isGeography = false;
-            else if (baseType.Equals("geography", StringComparison.OrdinalIgnoreCase))
+            else if (baseType.Equals("GEOGRAPHY", StringComparison.OrdinalIgnoreCase))
                 isGeography = true;
             else
                 return false;
@@ -83,19 +99,43 @@ public static bool TryParseStoreTypeName(string storeTypeName, out bool isGeogra
             if (closeParen != storeTypeName.Length - 1)
                 return false;
 
-            string subTypeString;
             var comma = storeTypeName.IndexOf(",", openParen + 1, StringComparison.Ordinal);
             if (comma == -1)
-                subTypeString = storeTypeName.Substring(openParen + 1, closeParen - openParen - 1).Trim();
+                subtypeName = storeTypeName.Substring(openParen + 1, closeParen - openParen - 1).Trim();
             else
             {
-                subTypeString = storeTypeName.Substring(openParen + 1, comma - openParen - 1).Trim();
+                subtypeName = storeTypeName.Substring(openParen + 1, comma - openParen - 1).Trim();
 
                 if (!int.TryParse(storeTypeName.Substring(comma + 1, closeParen - comma - 1).Trim(), out srid))
                     return false;
             }
 
-            return SubTypeNameToClrType.TryGetValue(subTypeString, out clrType);
+            subtypeName = subtypeName.ToUpper();
+
+            // We have geometry(subtype, srid), parse the subtype (POINT, POINTZ, POINTM, POINTZM...)
+
+            if (TryGetClrType(subtypeName, out clrType))
+                return true;
+
+            if (subtypeName.EndsWith("ZM") && TryGetClrType(subtypeName.Substring(0, subtypeName.Length - 2), out clrType))
+            {
+                ordinates = Ordinates.XYZM;
+                return true;
+            }
+
+            if (subtypeName.EndsWith("M") && TryGetClrType(subtypeName.Substring(0, subtypeName.Length - 1), out clrType))
+            {
+                ordinates = Ordinates.XYM;
+                return true;
+            }
+
+            if (subtypeName.EndsWith("Z") && TryGetClrType(subtypeName.Substring(0, subtypeName.Length - 1), out clrType))
+            {
+                ordinates = Ordinates.XYZ;
+                return true;
+            }
+
+            return false;
         }
     }
 }

src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs

@@ -263,6 +263,20 @@ protected override void Generate(
             if (IsSystemColumn(operation.Name))
                 return;
 
+            if (operation[NpgsqlAnnotationNames.ValueGenerationStrategy] is NpgsqlValueGenerationStrategy strategy)
+            {
+                switch (strategy)
+                {
+                case NpgsqlValueGenerationStrategy.SerialColumn:
+                case NpgsqlValueGenerationStrategy.IdentityAlwaysColumn:
+                case NpgsqlValueGenerationStrategy.IdentityByDefaultColumn:
+                    // NB: This gets added to all added non-nullable columns by MigrationsModelDiffer. We need to suppress
+                    // it, here because PG can't have both IDENTITY/SERIAL and a DEFAULT constraint on the same column.
+                    operation.DefaultValue = null;
+                    break;
+                }
+            }
+
             base.Generate(operation, model, builder, terminate: false);
 
             if (operation.Comment != null)
@@ -1080,6 +1094,9 @@ protected override void ColumnDefinition(
             var valueGenerationStrategy = operation[NpgsqlAnnotationNames.ValueGenerationStrategy] as NpgsqlValueGenerationStrategy?;
             if (valueGenerationStrategy == NpgsqlValueGenerationStrategy.SerialColumn)
             {
+                if (operation.IsNullable)
+                    throw new NotSupportedException("SERIAL columns can't be nullable");
+
                 switch (operation.ColumnType)
                 {
                 case "int":

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlBitTypeMapping.cs

@@ -30,7 +30,7 @@ protected override string GenerateNonNullSqlLiteral(object value)
         {
             var bits = (BitArray)value;
             var sb = new StringBuilder();
-            sb.Append("BIT B'");
+            sb.Append("B'");
             for (var i = 0; i < bits.Count; i++)
                 sb.Append(bits[i] ? '1' : '0');
             sb.Append('\'');

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlCharacterTypeMapping.cs

@@ -20,29 +20,34 @@ public class NpgsqlCharacterTypeMapping : StringTypeMapping
         /// Static <see cref="ValueComparer{T}"/> for fixed-width character types.
         /// </summary>
         /// <remarks>
-        /// Comparisons of 'character' data as defined in the SQL standard
-        /// differ dramatically from CLR string comparisons. This value comparer
-        /// adjusts for this by only comparing strings after truncating trailing
+        /// <p>
+        /// Comparisons of 'character' data as defined in the SQL standard differ dramatically from CLR string
+        /// comparisons. This value comparer adjusts for this by only comparing strings after truncating trailing
         /// whitespace.
+        /// </p>
+        /// <p>
+        /// Note that if a value converter is used and the CLR type isn't a string at all, we just use the default
+        /// value converter instead.
+        /// </p>
         /// </remarks>
         [NotNull] static readonly ValueComparer<string> CharacterValueComparer =
             new ValueComparer<string>(
                 (x, y) => EqualsWithoutTrailingWhitespace(x, y),
                 x => GetHashCodeWithoutTrailingWhitespace(x));
 
-        public override ValueComparer Comparer => CharacterValueComparer;
+        public override ValueComparer Comparer => ClrType == typeof(string) ? CharacterValueComparer : base.Comparer;
 
-        public override ValueComparer KeyComparer => CharacterValueComparer;
+        public override ValueComparer KeyComparer => Comparer;
 
         public NpgsqlCharacterTypeMapping([NotNull] string storeType, int? size = null)
             : this(new RelationalTypeMappingParameters(
                 new CoreTypeMappingParameters(typeof(string)),
                 storeType,
                 size == null ? StoreTypePostfix.None : StoreTypePostfix.Size,
                 System.Data.DbType.StringFixedLength,
-                false,
+                unicode: false,
                 size,
-                true)) {}
+                fixedLength: true)) {}
 
         protected NpgsqlCharacterTypeMapping(RelationalTypeMappingParameters parameters) : base(parameters) {}
 

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlVarbitTypeMapping.cs

@@ -21,7 +21,7 @@ protected override string GenerateNonNullSqlLiteral(object value)
         {
             var bits = (BitArray)value;
             var sb = new StringBuilder();
-            sb.Append("VARBIT B'");
+            sb.Append("B'");
             for (var i = 0; i < bits.Count; i++)
                 sb.Append(bits[i] ? '1' : '0');
             sb.Append('\'');

test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs

@@ -469,7 +469,7 @@ public void AddColumnOperation_serial_with_default()
                 });
 
             AssertSql(
-                @"ALTER TABLE ""People"" ADD foo serial NOT NULL DEFAULT 0;
+                @"ALTER TABLE ""People"" ADD foo serial NOT NULL;
 ");
         }
 

test/EFCore.PG.FunctionalTests/Query/QueryBugTest.cs

@@ -1,10 +1,12 @@
 using System;
+using System.ComponentModel.DataAnnotations.Schema;
 using System.Linq;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 using Xunit;
 using Xunit.Abstractions;
 using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities;
+// ReSharper disable UnusedAutoPropertyAccessor.Local
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query
 {
@@ -18,7 +20,6 @@ public QueryBugsTest(NpgsqlFixture fixture, ITestOutputHelper testOutputHelper)
             //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
         }
 
-        // ReSharper disable once MemberCanBePrivate.Global
         protected NpgsqlFixture Fixture { get; }
 
         #region Bug278
@@ -46,26 +47,53 @@ NpgsqlTestStore CreateDatabase278()
                     ClearLog();
                 });
 
-        // ReSharper disable once MemberCanBePrivate.Global
         public enum ChannelCode { Code = 1 }
 
-        // ReSharper disable once MemberCanBePrivate.Global
         public class Bug278Entity
         {
-            // ReSharper disable once UnusedMember.Global
             public int Id { get; set; }
             public int[] ChannelCodes { get; set; }
         }
 
         class Bug278Context : DbContext
         {
             public Bug278Context(DbContextOptions options) : base(options) {}
-            // ReSharper disable once UnusedAutoPropertyAccessor.Local
             public DbSet<Bug278Entity> Entities { get; set; }
         }
 
         #endregion Bug278
 
+        #region Bug920
+
+        [Fact]
+        public void Bug920()
+        {
+            using var _ = CreateDatabase920();
+            using var context = new Bug920Context(_options);
+            context.Entities.Add(new Bug920Entity { Enum = Bug920Enum.Two });
+            context.SaveChanges();
+        }
+
+        NpgsqlTestStore CreateDatabase920()
+            => CreateTestStore(() => new Bug920Context(_options), context => ClearLog());
+
+        public enum Bug920Enum { One, Two }
+
+        public class Bug920Entity
+        {
+            public int Id { get; set; }
+            [Column(TypeName="char(3)")]
+            public Bug920Enum Enum { get; set; }
+        }
+
+        class Bug920Context : DbContext
+        {
+            public Bug920Context(DbContextOptions options) : base(options) {}
+            public DbSet<Bug920Entity> Entities { get; set; }
+        }
+
+        #endregion Bug920
+
         DbContextOptions _options;
 
         NpgsqlTestStore CreateTestStore<TContext>(

test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs

@@ -25,6 +25,9 @@ public class NpgsqlTypeMappingSourceTest
         [InlineData("geometry", typeof(Geometry))]
         [InlineData("geometry(Polygon)", typeof(Polygon))]
         [InlineData("geography(Point, 4326)", typeof(Point))]
+        [InlineData("geometry(pointz, 4326)", typeof(Point))]
+        [InlineData("geography(LineStringZM)", typeof(LineString))]
+        [InlineData("geometry(POLYGONM)", typeof(Polygon))]
         public void By_StoreType(string storeType, Type expectedClrType)
         {
             var mapping = Source.FindMapping(storeType);