More precise translation of Sum

PG returns different types for Sum, so apply conversions.

Closes #976

src/EFCore.PG/Query/Internal/NpgsqlSqlTranslatingExpressionVisitor.cs

@@ -5,6 +5,7 @@
 using System.Reflection;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Query;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
@@ -56,17 +57,32 @@ public override SqlExpression TranslateCount(Expression expression = null)
                     _sqlExpressionFactory.Function("COUNT", new[] { _sqlExpressionFactory.Fragment("*") }, typeof(long))),
                 typeof(int), _sqlExpressionFactory.FindMapping(typeof(int)));
 
-//            // In PostgreSQL SUM() doesn't return the same type as its argument for smallint, int and bigint.
-//            // Cast to get the same type.
-//            // http://www.postgresql.org/docs/current/static/functions-aggregate.html
-//            if (sqlFunctionExpression.FunctionName == "SUM")
-//        {
-//            if (sqlFunctionExpression.Type == typeof(int))
-//                Sql.Append("::INT");
-//            else if (sqlFunctionExpression.Type == typeof(short))
-//                Sql.Append("::SMALLINT");
-//            return expr;
-//        }
+        // In PostgreSQL SUM() doesn't return the same type as its argument for smallint, int and bigint.
+        // Cast to get the same type.
+        // http://www.postgresql.org/docs/current/static/functions-aggregate.html
+        public override SqlExpression TranslateSum(Expression expression)
+        {
+            var sqlExpression = expression as SqlExpression ??
+                                Translate(expression) ??
+                                throw new InvalidOperationException(CoreStrings.TranslationFailed(expression.Print()));
+
+            var inputType = sqlExpression.Type.UnwrapNullableType();
+
+            // Note that there is no Sum over short in LINQ
+            if (inputType == typeof(int))
+                return _sqlExpressionFactory.Convert(
+                    _sqlExpressionFactory.Function("SUM", new[] { sqlExpression }, typeof(long)),
+                    inputType,
+                    sqlExpression.TypeMapping);
+
+            if (inputType == typeof(long))
+                return _sqlExpressionFactory.Convert(
+                    _sqlExpressionFactory.Function("SUM", new[] { sqlExpression }, typeof(decimal)),
+                    inputType,
+                    sqlExpression.TypeMapping);
+
+            return _sqlExpressionFactory.Function("SUM", new[] { sqlExpression }, inputType, sqlExpression.TypeMapping);
+        }
 
         /// <inheritdoc />
         protected override Expression VisitUnary(UnaryExpression unaryExpression)

test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs

@@ -8,7 +8,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.EntityFrameworkCore;
-using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.TestUtilities;
@@ -16,7 +15,6 @@
 using Npgsql.EntityFrameworkCore.PostgreSQL.TestUtilities;
 using NpgsqlTypes;
 
-// ReSharper disable InconsistentNaming
 namespace Npgsql.EntityFrameworkCore.PostgreSQL
 {
     public class BuiltInDataTypesNpgsqlTest : BuiltInDataTypesTestBase<BuiltInDataTypesNpgsqlTest.BuiltInDataTypesNpgsqlFixture>
@@ -24,7 +22,10 @@ public class BuiltInDataTypesNpgsqlTest : BuiltInDataTypesTestBase<BuiltInDataTy
         // ReSharper disable once UnusedParameter.Local
         public BuiltInDataTypesNpgsqlTest(BuiltInDataTypesNpgsqlFixture fixture, ITestOutputHelper testOutputHelper)
             : base(fixture)
-            => Fixture.TestSqlLoggerFactory.Clear();
+        {
+            Fixture.TestSqlLoggerFactory.Clear();
+            //Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
+        }
 
         [Fact]
         public void Sql_translation_uses_type_mapper_when_constant()
@@ -38,12 +39,11 @@ var results
                         .ToList();
 
                 Assert.Equal(0, results.Count);
-                Assert.Equal(
+
+                AssertSql(
                     @"SELECT m.""Int""
 FROM ""MappedNullableDataTypes"" AS m
-WHERE (m.""TimeSpanAsTime"" = TIME '00:01:02') AND (m.""TimeSpanAsTime"" IS NOT NULL)",
-                    Sql,
-                    ignoreLineEndingDifferences: true);
+WHERE (m.""TimeSpanAsTime"" = TIME '00:01:02') AND (m.""TimeSpanAsTime"" IS NOT NULL)");
             }
         }
 
@@ -61,14 +61,13 @@ var results
                         .ToList();
 
                 Assert.Equal(0, results.Count);
-                Assert.Equal(
+
+                AssertSql(
                     @"@__timeSpan_0='02:01:00' (Nullable = true) (DbType = Object)
 
 SELECT m.""Int""
 FROM ""MappedNullableDataTypes"" AS m
-WHERE ((m.""TimeSpanAsTime"" = @__timeSpan_0) AND ((m.""TimeSpanAsTime"" IS NOT NULL) AND (@__timeSpan_0 IS NOT NULL))) OR ((m.""TimeSpanAsTime"" IS NULL) AND (@__timeSpan_0 IS NULL))",
-                    Sql,
-                    ignoreLineEndingDifferences: true);
+WHERE ((m.""TimeSpanAsTime"" = @__timeSpan_0) AND ((m.""TimeSpanAsTime"" IS NOT NULL) AND (@__timeSpan_0 IS NOT NULL))) OR ((m.""TimeSpanAsTime"" IS NULL) AND (@__timeSpan_0 IS NULL))");
             }
         }
 
@@ -506,7 +505,7 @@ public virtual void Can_insert_and_read_back_all_mapped_data_types()
         }
 
         string DumpParameters()
-            => Fixture.TestSqlLoggerFactory.Parameters.Single().Replace(", ", EOL);
+            => Fixture.TestSqlLoggerFactory.Parameters.Single().Replace(", ", Environment.NewLine);
 
         // ReSharper disable once UnusedMember.Local
         // ReSharper disable once ParameterOnlyUsedForPreconditionCheck.Local
@@ -874,9 +873,31 @@ public override void Can_query_with_null_parameters_using_any_nullable_data_type
             }
         }
 
-        string Sql => Fixture.TestSqlLoggerFactory.Sql;
+        [ConditionalFact]
+        public void Sum_Conversions()
+        {
+            using (var context = CreateContext())
+            {
+                // PostgreSQL SUM() returns numeric for bigint input, bigint for int/smallint inuts.
+                // Make sure the proper conversion is done
+                var sum1 = context.Set<MappedDataTypes>().Sum(m => m.LongAsBigint);
+                var sum2 = context.Set<MappedDataTypes>().Sum(m => m.Int);
+                var sum3 = context.Set<MappedDataTypes>().Sum(m => m.ShortAsSmallint);
+
+                AssertSql(
+                    @"SELECT SUM(m.""LongAsBigint"")::bigint
+FROM ""MappedDataTypes"" AS m",
+                    //
+                    @"SELECT SUM(m.""Int"")::INT
+FROM ""MappedDataTypes"" AS m",
+                    //
+                    @"SELECT SUM(CAST(m.""ShortAsSmallint"" AS integer))::INT
+FROM ""MappedDataTypes"" AS m");
+            }
+        }
 
-        static readonly string EOL = Environment.NewLine;
+        void AssertSql(params string[] expected)
+            => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
 
         public class BuiltInDataTypesNpgsqlFixture : BuiltInDataTypesFixtureBase
         {