From 1ae77456ccb5131a328eba6b119b5aafc008c0fa Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Fri, 8 Jul 2022 15:42:46 +0200 Subject: [PATCH] Implement spatial EnvelopeCombine aggregate translation Closes #28184 --- ...tTopologySuiteAggregateMethodTranslator.cs | 7 +- ...uiteAggregateMethodCallTranslatorPlugin.cs | 6 +- ...tTopologySuiteAggregateMethodTranslator.cs | 90 +++++++++++-------- .../Query/SpatialQueryTestBase.cs | 16 ++++ .../SpatialQuerySqlServerGeographyTest.cs | 5 ++ .../SpatialQuerySqlServerGeometryTest.cs | 11 +++ .../Query/SpatialQuerySqliteTest.cs | 11 +++ 7 files changed, 104 insertions(+), 42 deletions(-) diff --git a/src/EFCore.SqlServer.NTS/Query/Internal/SqlServerNetTopologySuiteAggregateMethodTranslator.cs b/src/EFCore.SqlServer.NTS/Query/Internal/SqlServerNetTopologySuiteAggregateMethodTranslator.cs index 0d9328191cb..03b950d07a4 100644 --- a/src/EFCore.SqlServer.NTS/Query/Internal/SqlServerNetTopologySuiteAggregateMethodTranslator.cs +++ b/src/EFCore.SqlServer.NTS/Query/Internal/SqlServerNetTopologySuiteAggregateMethodTranslator.cs @@ -26,6 +26,9 @@ private static readonly MethodInfo ConvexHullMethod private static readonly MethodInfo UnionMethod = typeof(UnaryUnionOp).GetRuntimeMethod(nameof(UnaryUnionOp.Union), new[] { typeof(IEnumerable) })!; + private static readonly MethodInfo EnvelopeCombineMethod + = typeof(EnvelopeCombiner).GetRuntimeMethod(nameof(EnvelopeCombiner.CombineAsGeometry), new[] { typeof(IEnumerable) })!; + private readonly ISqlExpressionFactory _sqlExpressionFactory; private readonly IRelationalTypeMappingSource _typeMappingSource; @@ -72,7 +75,9 @@ public SqlServerNetTopologySuiteAggregateMethodTranslator( ? "UnionAggregate" : method == ConvexHullMethod ? "ConvexHullAggregate" - : null; + : method == EnvelopeCombineMethod + ? "EnvelopeAggregate" + : null; if (functionName is null) { diff --git a/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin.cs b/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin.cs index b451a8ba6a6..37e86e39037 100644 --- a/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin.cs +++ b/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin.cs @@ -17,13 +17,11 @@ public class SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin : IAggreg /// 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. /// - public SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin( - ISqlExpressionFactory sqlExpressionFactory, - IRelationalTypeMappingSource typeMappingSource) + public SqliteNetTopologySuiteAggregateMethodCallTranslatorPlugin(ISqlExpressionFactory sqlExpressionFactory) { Translators = new IAggregateMethodCallTranslator[] { - new SqliteNetTopologySuiteAggregateMethodTranslator(sqlExpressionFactory, typeMappingSource) + new SqliteNetTopologySuiteAggregateMethodTranslator(sqlExpressionFactory) }; } diff --git a/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodTranslator.cs b/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodTranslator.cs index 598ac741138..ceb53cbe775 100644 --- a/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodTranslator.cs +++ b/src/EFCore.Sqlite.NTS/Query/Internal/SqliteNetTopologySuiteAggregateMethodTranslator.cs @@ -26,8 +26,10 @@ private static readonly MethodInfo ConvexHullMethod private static readonly MethodInfo UnionMethod = typeof(UnaryUnionOp).GetRuntimeMethod(nameof(UnaryUnionOp.Union), new[] { typeof(IEnumerable) })!; + private static readonly MethodInfo EnvelopeCombineMethod + = typeof(EnvelopeCombiner).GetRuntimeMethod(nameof(EnvelopeCombiner.CombineAsGeometry), new[] { typeof(IEnumerable) })!; + private readonly ISqlExpressionFactory _sqlExpressionFactory; - private readonly IRelationalTypeMappingSource _typeMappingSource; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -35,13 +37,8 @@ private static readonly MethodInfo UnionMethod /// 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. /// - public SqliteNetTopologySuiteAggregateMethodTranslator( - ISqlExpressionFactory sqlExpressionFactory, - IRelationalTypeMappingSource typeMappingSource) - { - _sqlExpressionFactory = sqlExpressionFactory; - _typeMappingSource = typeMappingSource; - } + public SqliteNetTopologySuiteAggregateMethodTranslator(ISqlExpressionFactory sqlExpressionFactory) + => _sqlExpressionFactory = sqlExpressionFactory; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -53,48 +50,67 @@ public SqliteNetTopologySuiteAggregateMethodTranslator( MethodInfo method, EnumerableExpression source, IReadOnlyList arguments, IDiagnosticsLogger logger) { - if (source.Selector is not SqlExpression sqlExpression - || (method != GeometryCombineMethod && method != UnionMethod && method != ConvexHullMethod)) + if (source.Selector is not SqlExpression sqlExpression) { return null; } - if (source.Predicate != null) + if (method == ConvexHullMethod) { - sqlExpression = _sqlExpressionFactory.Case( - new List { new(source.Predicate, sqlExpression) }, - elseResult: null); - } + CombineAggregateTerms(); - if (source.IsDistinct) - { - sqlExpression = new DistinctExpression(sqlExpression); - } - - if (method == GeometryCombineMethod || method == UnionMethod) - { + // Spatialite has no built-in aggregate convex hull, but we can simply apply Collect beforehand return _sqlExpressionFactory.Function( - method == GeometryCombineMethod ? "Collect" : "GUnion", - new[] { sqlExpression }, + "ConvexHull", + new[] + { + _sqlExpressionFactory.Function( + "Collect", + new[] { sqlExpression }, + nullable: true, + argumentsPropagateNullability: new[] { false }, + typeof(Geometry)) + }, nullable: true, - argumentsPropagateNullability: new[] { false }, + argumentsPropagateNullability: new[] { true }, typeof(Geometry)); } - // Spatialite has no built-in aggregate convex hull, but we can simply apply Collect beforehand + var functionName = method == UnionMethod + ? "GUnion" + : method == GeometryCombineMethod + ? "Collect" + : method == EnvelopeCombineMethod + ? "Extent" + : null; + + if (functionName is null) + { + return null; + } + + CombineAggregateTerms(); + return _sqlExpressionFactory.Function( - "ConvexHull", - new[] - { - _sqlExpressionFactory.Function( - "Collect", - new[] { sqlExpression }, - nullable: true, - argumentsPropagateNullability: new[] { false }, - typeof(Geometry)) - }, + functionName, + new[] { sqlExpression }, nullable: true, - argumentsPropagateNullability: new[] { true }, + argumentsPropagateNullability: new[] { false }, typeof(Geometry)); + + void CombineAggregateTerms() + { + if (source.Predicate != null) + { + sqlExpression = _sqlExpressionFactory.Case( + new List { new(source.Predicate, sqlExpression) }, + elseResult: null); + } + + if (source.IsDistinct) + { + sqlExpression = new DistinctExpression(sqlExpression); + } + } } } diff --git a/test/EFCore.Specification.Tests/Query/SpatialQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/SpatialQueryTestBase.cs index 377771a78f9..5d6372135af 100644 --- a/test/EFCore.Specification.Tests/Query/SpatialQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/SpatialQueryTestBase.cs @@ -208,6 +208,22 @@ public virtual Task Combine_aggregate(bool async) Assert.Equal(eCollection.Geometries, aCollection.Geometries); }); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task EnvelopeCombine_aggregate(bool async) + => AssertQuery( + async, + ss => ss.Set() + .Where(e => e.Point != null) + .GroupBy(e => e.Group) + .Select(g => new { Id = g.Key, Combined = EnvelopeCombiner.CombineAsGeometry(g.Select(e => e.Point)) }), + elementSorter: x => x.Id, + elementAsserter: (e, a) => + { + Assert.Equal(e.Id, a.Id); + Assert.Equal(e.Combined, a.Combined, GeometryComparer.Instance); + }); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Contains(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeographyTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeographyTest.cs index df2209c1d21..93732d8f9ae 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeographyTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeographyTest.cs @@ -3,6 +3,7 @@ using Microsoft.EntityFrameworkCore.TestModels.SpatialModel; using NetTopologySuite.Geometries; +using NetTopologySuite.IO; namespace Microsoft.EntityFrameworkCore.Query; @@ -120,6 +121,10 @@ WHERE [p].[Point] IS NOT NULL GROUP BY [p].[Group]"); } + // SQL Server returns a CurvePolygon, https://github.com/NetTopologySuite/NetTopologySuite.IO.SqlServerBytes/issues/18 + public override async Task EnvelopeCombine_aggregate(bool async) + => await Assert.ThrowsAsync(() => base.EnvelopeCombine_aggregate(async)); + public override async Task Contains(bool async) { await base.Contains(async); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeometryTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeometryTest.cs index d50d4b39fbb..9ba9feb3fb3 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeometryTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/SpatialQuerySqlServerGeometryTest.cs @@ -126,6 +126,17 @@ WHERE [p].[Point] IS NOT NULL GROUP BY [p].[Group]"); } + public override async Task EnvelopeCombine_aggregate(bool async) + { + await base.EnvelopeCombine_aggregate(async); + + AssertSql( + @"SELECT [p].[Group] AS [Id], geometry::EnvelopeAggregate([p].[Point]) AS [Combined] +FROM [PointEntity] AS [p] +WHERE [p].[Point] IS NOT NULL +GROUP BY [p].[Group]"); + } + public override async Task Contains(bool async) { await base.Contains(async); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/SpatialQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/SpatialQuerySqliteTest.cs index 98a8031b51a..85244dea44e 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/SpatialQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/SpatialQuerySqliteTest.cs @@ -166,6 +166,17 @@ public override async Task Combine_aggregate(bool async) GROUP BY ""p"".""Group"""); } + public override async Task EnvelopeCombine_aggregate(bool async) + { + await base.EnvelopeCombine_aggregate(async); + + AssertSql( + @"SELECT ""p"".""Group"" AS ""Id"", Extent(""p"".""Point"") AS ""Combined"" +FROM ""PointEntity"" AS ""p"" +WHERE ""p"".""Point"" IS NOT NULL +GROUP BY ""p"".""Group"""); + } + public override async Task Contains(bool async) { await base.Contains(async);