Skip to content

Commit

Permalink
Fixes for 3.1.0-preview3
Browse files Browse the repository at this point in the history
Fixes #1103
  • Loading branch information
roji committed Nov 7, 2019
1 parent 4674416 commit 7ab6383
Show file tree
Hide file tree
Showing 19 changed files with 79 additions and 155 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage;
using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal;
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;

namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal
{
Expand Down Expand Up @@ -51,18 +53,22 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
return null;

var operand = arguments[0];
Type operandElementType;

if (operand.Type.IsArray)
{
operandElementType = operand.Type.GetElementType();
}
else if (operand.Type.IsGenericType && operand.Type.GetGenericTypeDefinition() == typeof(List<>))
var operandElementType = operand.Type.IsArray
? operand.Type.GetElementType()
: operand.Type.IsGenericType && operand.Type.GetGenericTypeDefinition() == typeof(List<>)
? operand.Type.GetGenericArguments()[0]
: null;

if (operandElementType == null) // Not an array/list
return null;

// Even if the CLR type is an array/list, it may be mapped to a non-array database type (e.g. via value converters).
if (operand.TypeMapping is RelationalTypeMapping typeMapping &&
!(typeMapping is NpgsqlArrayTypeMapping) && !(typeMapping is NpgsqlJsonTypeMapping))
{
operandElementType = operand.Type.GetGenericArguments()[0];
return null;
}
else
return null; // Not an array/list

if (method.IsClosedFormOf(SequenceEqual) && arguments[1].Type.IsArray)
return _sqlExpressionFactory.Equal(operand, arguments[1]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,15 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
nameof(NpgsqlFullTextSearchLinqExtensions.And) => QueryReturningOnTwoQueries("&&"),
nameof(NpgsqlFullTextSearchLinqExtensions.Or) => QueryReturningOnTwoQueries("||"),

nameof(NpgsqlFullTextSearchLinqExtensions.ToNegative) => new SqlUnaryExpression(ExpressionType.Not, arguments[0], arguments[0].Type, arguments[0].TypeMapping),
// TODO: Hack, see #1118
nameof(NpgsqlFullTextSearchLinqExtensions.ToNegative)
=> new SqlUnaryExpression(ExpressionType.Negate, arguments[0], arguments[0].Type, arguments[0].TypeMapping),

nameof(NpgsqlFullTextSearchLinqExtensions.Contains) => BoolReturningOnTwoQueries("@>"),
nameof(NpgsqlFullTextSearchLinqExtensions.IsContainedIn) => BoolReturningOnTwoQueries("<@"),

nameof(NpgsqlFullTextSearchLinqExtensions.Concat) => _sqlExpressionFactory.Add(arguments[0], arguments[1], _tsVectorMapping),
nameof(NpgsqlFullTextSearchLinqExtensions.Concat)
=> _sqlExpressionFactory.Add(arguments[0], arguments[1], _tsVectorMapping),

nameof(NpgsqlFullTextSearchLinqExtensions.Matches) => new SqlCustomBinaryExpression(
_sqlExpressionFactory.ApplyDefaultTypeMapping(arguments[0]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadO
nameof(NpgsqlNetworkExtensions.ContainsOrEqual) => BoolReturningOnTwoNetworkTypes(">>="),
nameof(NpgsqlNetworkExtensions.ContainsOrContainedBy) => BoolReturningOnTwoNetworkTypes("&&"),

nameof(NpgsqlNetworkExtensions.BitwiseNot) => new SqlUnaryExpression(ExpressionType.Not,
// TODO: Hack, see #1118
nameof(NpgsqlNetworkExtensions.BitwiseNot) => new SqlUnaryExpression(ExpressionType.Negate,
arguments[1],
arguments[1].Type,
arguments[1].TypeMapping),
Expand Down
6 changes: 4 additions & 2 deletions src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
}

// Bitwise complement on networking types
case ExpressionType.Not when
// TODO: Hack, see #1118
case ExpressionType.Negate when
sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(IPAddress) ||
sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof((IPAddress, int)) ||
sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(PhysicalAddress):
Expand All @@ -202,7 +203,8 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
return sqlUnaryExpression;

// Negation on full-text queries
case ExpressionType.Not when sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(NpgsqlTsQuery):
// TODO: Hack, see #1118
case ExpressionType.Negate when sqlUnaryExpression.Operand.TypeMapping.ClrType == typeof(NpgsqlTsQuery):
Sql.Append("!!");
Visit(sqlUnaryExpression.Operand);
return sqlUnaryExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,18 @@ public NpgsqlSqlTranslatingExpressionVisitor(
// PostgreSQL COUNT() always returns bigint, so we need to downcast to int
// TODO: Translate Count with predicate for GroupBy (see base implementation)
public override SqlExpression TranslateCount(Expression expression = null)
=> _sqlExpressionFactory.Convert(
{
if (expression != null)
{
// TODO: Translate Count with predicate for GroupBy
return null;
}

return _sqlExpressionFactory.Convert(
_sqlExpressionFactory.ApplyDefaultTypeMapping(
_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.
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void Sql_translation_uses_type_mapper_when_constant()
AssertSql(
@"SELECT m.""Int""
FROM ""MappedNullableDataTypes"" AS m
WHERE (m.""TimeSpanAsTime"" = TIME '00:01:02') AND (m.""TimeSpanAsTime"" IS NOT NULL)");
WHERE m.""TimeSpanAsTime"" = TIME '00:01:02'");
}

[Fact]
Expand All @@ -61,7 +61,7 @@ public void Sql_translation_uses_type_mapper_when_parameter()
SELECT m.""Int""
FROM ""MappedNullableDataTypes"" AS m
WHERE (m.""TimeSpanAsTime"" = @__timeSpan_0) AND (m.""TimeSpanAsTime"" IS NOT NULL)");
WHERE m.""TimeSpanAsTime"" = @__timeSpan_0");
}

[Fact]
Expand Down
26 changes: 13 additions & 13 deletions test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void Index_with_non_constant()
SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE (s.""SomeArray""[@__x_0 + 1] = 3) AND (s.""SomeArray""[@__x_0 + 1] IS NOT NULL)");
WHERE s.""SomeArray""[@__x_0 + 1] = 3");
}

[Fact]
Expand All @@ -75,7 +75,7 @@ public void Index_bytea_with_constant()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE (get_byte(s.""SomeBytea"", 0) = 3) AND (get_byte(s.""SomeBytea"", 0) IS NOT NULL)");
WHERE get_byte(s.""SomeBytea"", 0) = 3");
}

[Fact(Skip = "Disabled since EF Core 3.0")]
Expand Down Expand Up @@ -108,7 +108,7 @@ public void SequenceEqual_with_parameter()
SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE (s.""SomeArray"" = @__arr_0) AND (s.""SomeArray"" IS NOT NULL)
WHERE s.""SomeArray"" = @__arr_0
LIMIT 2");
}

Expand All @@ -122,15 +122,15 @@ public void SequenceEqual_with_array_literal()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE (s.""SomeArray"" = ARRAY[3,4]::integer[]) AND (s.""SomeArray"" IS NOT NULL)
WHERE s.""SomeArray"" = ARRAY[3,4]::integer[]
LIMIT 2");
}

#endregion

#region Containment

[Fact(Skip = "https://github.com/aspnet/EntityFrameworkCore/issues/17374")]
[Fact]
public void Contains_with_literal()
{
using var ctx = Fixture.CreateContext();
Expand All @@ -140,11 +140,11 @@ public void Contains_with_literal()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE 3 = ANY (s.""SomeArray"")
WHERE COALESCE(3 = ANY (s.""SomeArray""), FALSE)
LIMIT 2");
}

[Fact(Skip = "https://github.com/aspnet/EntityFrameworkCore/issues/17374")]
[Fact]
public void Contains_with_parameter()
{
using var ctx = Fixture.CreateContext();
Expand All @@ -158,11 +158,11 @@ public void Contains_with_parameter()
SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE @__p_0 = ANY (s.""SomeArray"")
WHERE COALESCE(@__p_0 = ANY (s.""SomeArray""), FALSE)
LIMIT 2");
}

[Fact(Skip = "https://github.com/aspnet/EntityFrameworkCore/issues/17374")]
[Fact]
public void Contains_with_column()
{
using var ctx = Fixture.CreateContext();
Expand All @@ -172,7 +172,7 @@ public void Contains_with_column()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE s.""Id"" + 2 = ANY (s.""SomeArray"")
WHERE COALESCE(s.""Id"" + 2 = ANY (s.""SomeArray""), FALSE)
LIMIT 2");
}

Expand All @@ -190,7 +190,7 @@ public void Length()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE (cardinality(s.""SomeArray"") = 2) AND (cardinality(s.""SomeArray"") IS NOT NULL)
WHERE cardinality(s.""SomeArray"") = 2
LIMIT 2");
}

Expand All @@ -204,7 +204,7 @@ public void Length_on_EF_Property()
AssertSql(
@"SELECT s.""Id"", s.""SomeArray"", s.""SomeBytea"", s.""SomeList"", s.""SomeMatrix"", s.""SomeText""
FROM ""SomeEntities"" AS s
WHERE (cardinality(s.""SomeArray"") = 2) AND (cardinality(s.""SomeArray"") IS NOT NULL)
WHERE cardinality(s.""SomeArray"") = 2
LIMIT 2");
}

Expand All @@ -221,7 +221,7 @@ public void Length_on_literal_not_translated()

#region AnyAll

[Fact(Skip = "https://github.com/aspnet/EntityFrameworkCore/issues/17374")]
[Fact]
public void Any_no_predicate()
{
using var ctx = Fixture.CreateContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@ public ComplexNavigationsQueryNpgsqlTest(ComplexNavigationsQueryNpgsqlFixture fi
Fixture.TestSqlLoggerFactory.Clear();
}

[ConditionalTheory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/18679")]
[MemberData(nameof(IsAsyncData))]
public override Task Include13(bool isAsync) => base.Include13(isAsync);

[ConditionalTheory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/18679")]
[MemberData(nameof(IsAsyncData))]
public override Task Include14(bool isAsync) => base.Include13(isAsync);

// Should be fixed but could not verify as temporarily disabled upstream
// [ConditionalTheory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/12970")]
// [MemberData(nameof(IsAsyncData))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,5 @@ public ComplexNavigationsWeakQueryNpgsqlTest(ComplexNavigationsWeakQueryNpgsqlFi
Fixture.TestSqlLoggerFactory.Clear();
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

[ConditionalTheory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/18679")]
[MemberData(nameof(IsAsyncData))]
public override Task Include13(bool isAsync) => base.Include13(isAsync);

[ConditionalTheory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/18679")]
[MemberData(nameof(IsAsyncData))]
public override Task Include14(bool isAsync) => base.Include13(isAsync);
}
}
10 changes: 0 additions & 10 deletions test/EFCore.PG.FunctionalTests/Query/GearsOfWarQueryNpgsqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,6 @@ public GearsOfWarQueryNpgsqlTest(GearsOfWarQueryNpgsqlFixture fixture, ITestOutp
public override Task String_concat_with_null_conditional_argument2(bool isAsync)
=> base.String_concat_with_null_conditional_argument2(isAsync);

[Theory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/18674")]
[MemberData(nameof(IsAsyncData))]
public override Task Concat_with_collection_navigations(bool isAsync)
=> base.Concat_with_collection_navigations(isAsync);

[Theory(Skip = "https://github.com/aspnet/EntityFrameworkCore/pull/18674")]
[MemberData(nameof(IsAsyncData))]
public override Task Select_navigation_with_concat_and_count(bool isAsync)
=> base.Select_navigation_with_concat_and_count(isAsync);

#region Ignore DateTimeOffset tests

// PostgreSQL has no datatype that corresponds to DateTimeOffset.
Expand Down
35 changes: 6 additions & 29 deletions test/EFCore.PG.FunctionalTests/Query/GroupByQueryNpgsqlTest.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.TestModels.Northwind;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -19,34 +17,13 @@ public GroupByQueryNpgsqlTest(NorthwindQueryNpgsqlFixture<NoopModelCustomizer> f
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

[ConditionalTheory] // https://github.com/aspnet/EntityFrameworkCore/pull/18675
[MemberData(nameof(IsAsyncData))]
public override Task GroupBy_aggregate_projecting_conditional_expression(bool isAsync)
{
return AssertQuery(
isAsync,
ss => ss.Set<Order>().GroupBy(o => o.OrderDate).Select(
g =>
new { g.Key, SomeValue = g.Count() == 0 ? 1 : g.Sum(o => o.OrderID % 2 == 0 ? 1 : 0) / g.Count() }),
e => (e.Key, e.SomeValue));
}
public override Task GroupBy_Property_Select_Count_with_predicate(bool isAsync)
=> Assert.ThrowsAsync<InvalidOperationException>(
() => base.GroupBy_Property_Select_Count_with_predicate(isAsync));

[ConditionalTheory] // https://github.com/aspnet/EntityFrameworkCore/pull/18675
[MemberData(nameof(IsAsyncData))]
public override Task GroupBy_with_order_by_skip_and_another_order_by(bool isAsync)
{
return AssertQueryScalar(
isAsync,
ss => ss.Set<Order>()
.OrderBy(o => o.CustomerID)
.ThenBy(o => o.OrderID)
.Skip(80)
.OrderBy(o => o.CustomerID)
.ThenBy(o => o.OrderID)
.GroupBy(o => o.CustomerID)
.Select(g => g.Sum(o => o.OrderID))
);
}
public override Task GroupBy_Property_Select_LongCount_with_predicate(bool isAsync)
=> Assert.ThrowsAsync<InvalidOperationException>(
() => base.GroupBy_Property_Select_LongCount_with_predicate(isAsync));

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();
Expand Down
24 changes: 4 additions & 20 deletions test/EFCore.PG.FunctionalTests/Query/JsonDomQueryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void Literal_document()
AssertSql(
@"SELECT j.""Id"", j.""CustomerDocument"", j.""CustomerElement""
FROM ""JsonbEntities"" AS j
WHERE (j.""CustomerDocument"" = '{""Name"":""Test customer"",""Age"":80}') AND (j.""CustomerDocument"" IS NOT NULL)");
WHERE j.""CustomerDocument"" = '{""Name"":""Test customer"",""Age"":80}'");
}

[Fact]
Expand All @@ -87,7 +87,7 @@ public void Parameter_document()
SELECT j.""Id"", j.""CustomerDocument"", j.""CustomerElement""
FROM ""JsonbEntities"" AS j
WHERE (j.""CustomerDocument"" = @__expected_0) AND (j.""CustomerDocument"" IS NOT NULL)
WHERE j.""CustomerDocument"" = @__expected_0
LIMIT 2");
}

Expand Down Expand Up @@ -284,7 +284,7 @@ public void Array_parameter_index()
SELECT j.""Id"", j.""CustomerDocument"", j.""CustomerElement""
FROM ""JsonbEntities"" AS j
WHERE (CAST(j.""CustomerElement""#>>ARRAY['Statistics','Nested','IntArray',@__i_0]::TEXT[] AS integer) = 4) AND (CAST(j.""CustomerElement""#>>ARRAY['Statistics','Nested','IntArray',@__i_0]::TEXT[] AS integer) IS NOT NULL)
WHERE CAST(j.""CustomerElement""#>>ARRAY['Statistics','Nested','IntArray',@__i_0]::TEXT[] AS integer) = 4
LIMIT 2");
}

Expand Down Expand Up @@ -316,22 +316,6 @@ WHERE json_array_length(j.""CustomerElement""->'Orders') = 2
LIMIT 2");
}

#if ISSUE_17374
[Fact(Skip = "https://github.com/aspnet/EntityFrameworkCore/issues/17374")]
public void Array_Any_toplevel()
{
using var ctx = Fixture.CreateContext();
var x = ctx.JsonbEntities.Single(e => e.ToplevelArray.Any());

Assert.Equal("Joe", x.Customer.Name);
AssertSql(
@"SELECT j.""Id"", j.""Customer""
FROM ""JsonbEntities"" AS j
WHERE jsonb_array_length(j.""ToplevelArray"") > 0
LIMIT 2");
}
#endif

[Fact]
public void Like()
{
Expand All @@ -342,7 +326,7 @@ public void Like()
AssertSql(
@"SELECT j.""Id"", j.""CustomerDocument"", j.""CustomerElement""
FROM ""JsonbEntities"" AS j
WHERE j.""CustomerElement""->>'Name' LIKE 'J%'
WHERE (j.""CustomerElement""->>'Name' IS NOT NULL) AND (j.""CustomerElement""->>'Name' LIKE 'J%')
LIMIT 2");
}

Expand Down
Loading

0 comments on commit 7ab6383

Please sign in to comment.