From d5cf5bddbca93cfbfd235ee0d9e80dbc06ba159c Mon Sep 17 00:00:00 2001
From: Mark Carrington <31017244+MarkMpn@users.noreply.github.com>
Date: Mon, 8 Jul 2024 20:19:36 +0100
Subject: [PATCH] Use in-memory implementations for DISTINCT, OFFSET/FETCH,
 ORDER BY and TOP when working with virtual entity providers Fixes #503

---
 .../MarkMpn.Sql4Cds.Engine.Tests.csproj          | 16 ++++++----------
 .../ExecutionPlan/DistinctNode.cs                |  4 +++-
 .../ExecutionPlan/FetchXmlScan.cs                |  2 ++
 .../ExecutionPlan/OffsetFetchNode.cs             |  4 ++++
 MarkMpn.Sql4Cds.Engine/ExecutionPlan/SortNode.cs |  8 ++++++++
 MarkMpn.Sql4Cds.Engine/ExecutionPlan/TopNode.cs  |  5 +++++
 6 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/MarkMpn.Sql4Cds.Engine.Tests/MarkMpn.Sql4Cds.Engine.Tests.csproj b/MarkMpn.Sql4Cds.Engine.Tests/MarkMpn.Sql4Cds.Engine.Tests.csproj
index d502b328..f19791ce 100644
--- a/MarkMpn.Sql4Cds.Engine.Tests/MarkMpn.Sql4Cds.Engine.Tests.csproj
+++ b/MarkMpn.Sql4Cds.Engine.Tests/MarkMpn.Sql4Cds.Engine.Tests.csproj
@@ -110,16 +110,6 @@
     <None Include="app.config" />
     <None Include="Key.snk" />
   </ItemGroup>
-  <ItemGroup>
-    <ProjectReference Include="..\MarkMpn.Sql4Cds.Engine\MarkMpn.Sql4Cds.Engine.csproj">
-      <Project>{c77b731d-e55c-4197-b96c-2b23eb9f56ef}</Project>
-      <Name>MarkMpn.Sql4Cds.Engine</Name>
-    </ProjectReference>
-    <ProjectReference Include="..\MarkMpn.Sql4Cds\MarkMpn.Sql4Cds.csproj">
-      <Project>{a7af5d13-a44e-426d-b3fc-ae390832c7df}</Project>
-      <Name>MarkMpn.Sql4Cds</Name>
-    </ProjectReference>
-  </ItemGroup>
   <ItemGroup>
     <PackageReference Include="Dapper.StrongName">
       <Version>2.1.28</Version>
@@ -140,6 +130,12 @@
       <Version>6.0.7</Version>
     </PackageReference>
   </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\MarkMpn.Sql4Cds.Engine\MarkMpn.Sql4Cds.Engine.csproj">
+      <Project>{c77b731d-e55c-4197-b96c-2b23eb9f56ef}</Project>
+      <Name>MarkMpn.Sql4Cds.Engine</Name>
+    </ProjectReference>
+  </ItemGroup>
   <Import Project="$(VSToolsPath)\TeamTest\Microsoft.TestTools.targets" Condition="Exists('$(VSToolsPath)\TeamTest\Microsoft.TestTools.targets')" />
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
 </Project>
\ No newline at end of file
diff --git a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/DistinctNode.cs b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/DistinctNode.cs
index 058aeca5..98081d6c 100644
--- a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/DistinctNode.cs
+++ b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/DistinctNode.cs
@@ -119,7 +119,9 @@ public override IDataExecutionPlanNodeInternal FoldQuery(NodeCompilationContext
                     }
                 }
 
-                if (!virtualAttr)
+                // Virtual entity providers are unreliable - still fold the DISTINCT to the fetch but keep
+                // this node to ensure the DISTINCT is applied if the provider doesn't support it.
+                if (!virtualAttr && !fetch.IsUnreliableVirtualEntityProvider)
                     return fetch;
 
                 schema = Source.GetSchema(context);
diff --git a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
index ab176e92..42c9800c 100644
--- a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
+++ b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
@@ -196,6 +196,8 @@ public FetchXmlScan()
         [Browsable(false)]
         public string Warning => _missingPagingCookie && RowsOut == 50_000 ? "Using legacy paging - results may be incomplete" : null;
 
+        internal bool IsUnreliableVirtualEntityProvider => _isVirtualEntity;
+
         public bool RequiresCustomPaging(IDictionary<string, DataSource> dataSources)
         {
             // Never need to do paging if we're enforcing a TOP constraint
diff --git a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/OffsetFetchNode.cs b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/OffsetFetchNode.cs
index ed880e4d..afc53e5e 100644
--- a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/OffsetFetchNode.cs
+++ b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/OffsetFetchNode.cs
@@ -73,6 +73,10 @@ public override IDataExecutionPlanNodeInternal FoldQuery(NodeCompilationContext
 
             if (Source is FetchXmlScan fetchXml)
             {
+                // Virtual entity providers aren't reliable - use naive implementation
+                if (fetchXml.IsUnreliableVirtualEntityProvider)
+                    return this;
+
                 var expressionExecutionContext = new ExpressionExecutionContext(expressionCompilationContext);
                 var offset = SqlTypeConverter.ChangeType<int>(offsetLiteral.Compile(expressionCompilationContext)(expressionExecutionContext));
                 var count = SqlTypeConverter.ChangeType<int>(fetchLiteral.Compile(expressionCompilationContext)(expressionExecutionContext));
diff --git a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/SortNode.cs b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/SortNode.cs
index 938f53f5..eb7d6ce4 100644
--- a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/SortNode.cs
+++ b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/SortNode.cs
@@ -519,6 +519,14 @@ private IDataExecutionPlanNodeInternal FoldSorts(NodeCompilationContext context)
                         fetchXml.FetchXml.UseRawOrderBy = true;
                 }
 
+                // Virtual entity providers are unreliable - fold the sorts to the FetchXML but keep this
+                // node to resort if required.
+                if (fetchXml.IsUnreliableVirtualEntityProvider)
+                {
+                    PresortedCount = 0;
+                    return this;
+                }
+
                 return Source;
             }
 
diff --git a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/TopNode.cs b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/TopNode.cs
index 0f5d0044..05b96cb9 100644
--- a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/TopNode.cs
+++ b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/TopNode.cs
@@ -169,6 +169,11 @@ public override IDataExecutionPlanNodeInternal FoldQuery(NodeCompilationContext
                         fetchXml.FetchXml.top = literal.Value;
                         fetchXml.AllPages = false;
 
+                        // Virtual entity providers aren't reliable - fold the TOP into the FetchXML but keep
+                        // this node in case the provider doesn't support TOP
+                        if (fetchXml.IsUnreliableVirtualEntityProvider)
+                            return this;
+
                         if (Source == fetchXml)
                             return fetchXml;