From 11a48bccd234d18b287e31fdd9b9783a3bd2f685 Mon Sep 17 00:00:00 2001
From: Mark Carrington <31017244+MarkMpn@users.noreply.github.com>
Date: Wed, 11 Dec 2024 19:30:44 +0000
Subject: [PATCH] Performance improvements for queries with large number of
 filter conditions

---
 .../ExecutionPlanTests.cs                     | 131 ++++++++++++++++++
 .../ExecutionPlan/FetchXmlScan.cs             |  87 ++++++++++--
 MarkMpn.Sql4Cds.Engine/FetchXml.Extensions.cs |  66 +++++++++
 3 files changed, 269 insertions(+), 15 deletions(-)

diff --git a/MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs b/MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs
index c33baa34..9507eafb 100644
--- a/MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs
+++ b/MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Data;
 using System.Data.SqlTypes;
+using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using System.Linq;
@@ -9013,6 +9014,136 @@ public void InVariableAndLiteral()
       </condition>
     </filter>
   </entity>
+</fetch>");
+        }
+
+        [TestMethod]
+        public void LotsOfConditions()
+        {
+            var planBuilder = new ExecutionPlanBuilder(new SessionContext(_localDataSources, this), this);
+
+            var query = @"
+SELECT fullname 
+FROM contact 
+WHERE statecode = 0 
+      AND firstname <> 'Test1' 
+      AND firstname <> 'Test2' 
+      AND firstname <> 'Test3' 
+      AND firstname <> 'Test4' 
+      AND firstname <> 'Test5' 
+      AND firstname <> 'Test6' 
+      AND firstname <> 'Test7' 
+      AND firstname <> 'Test8' 
+      AND firstname <> 'Test9' 
+      AND firstname <> 'Test10' 
+      AND firstname <> 'Test11' 
+      AND firstname <> 'Test12' 
+      AND firstname <> 'Test13' 
+      AND firstname <> 'Test14' 
+      AND firstname <> 'Test15' 
+      AND firstname <> 'Test16' 
+      AND firstname <> 'Test17' 
+      AND firstname <> 'Test18' 
+      AND firstname <> 'Test19'
+      AND firstname <> 'Test20' 
+      AND firstname <> 'Test21' 
+      AND firstname <> 'Test22' 
+      AND firstname <> 'Test23' 
+      AND firstname <> 'Test24' 
+      AND firstname <> 'Test25' 
+      AND firstname <> 'Test26' 
+      AND firstname <> 'Test27' 
+      AND firstname <> 'Test28' 
+      AND firstname <> 'Test29'
+      AND firstname <> 'Test30' 
+      AND firstname <> 'Test31' 
+      AND firstname <> 'Test32' 
+      AND firstname <> 'Test33' 
+      AND firstname <> 'Test34' 
+      AND firstname <> 'Test35' 
+      AND firstname <> 'Test36' 
+      AND firstname <> 'Test37' 
+      AND firstname <> 'Test38' 
+      AND firstname <> 'Test39'
+      AND firstname <> 'Test40' 
+      AND firstname <> 'Test41' 
+      AND firstname <> 'Test42' 
+      AND firstname <> 'Test43' 
+      AND firstname <> 'Test44' 
+      AND firstname <> 'Test45' 
+      AND firstname <> 'Test46' 
+      AND firstname <> 'Test47' 
+      AND firstname <> 'Test48' 
+      AND firstname <> 'Test49'";
+
+            var timer = new Stopwatch();
+            timer.Start();
+            var plans = planBuilder.Build(query, null, out _);
+            timer.Stop();
+
+            Assert.IsTrue(timer.ElapsedMilliseconds < 2000, $"Query took {timer.ElapsedMilliseconds}ms");
+            Assert.AreEqual(1, plans.Length);
+
+            var select = AssertNode<SelectNode>(plans[0]);
+            var fetch = AssertNode<FetchXmlScan>(select.Source);
+            AssertFetchXml(fetch, @"
+<fetch xmlns:generator='MarkMpn.SQL4CDS'>
+  <entity name='contact'>
+    <attribute name='fullname' />
+    <filter>
+      <condition attribute='statecode' operator='eq' value='0' />
+      <condition attribute='firstname' operator='ne' value='Test1' />
+      <condition attribute='firstname' operator='not-null' />
+      <condition attribute='firstname' operator='ne' value='Test2' />
+        <condition attribute='firstname' operator='ne' value='Test3' />
+        <condition attribute='firstname' operator='ne' value='Test4' />
+        <condition attribute='firstname' operator='ne' value='Test5' />
+        <condition attribute='firstname' operator='ne' value='Test6' />
+        <condition attribute='firstname' operator='ne' value='Test7' />
+        <condition attribute='firstname' operator='ne' value='Test8' />
+        <condition attribute='firstname' operator='ne' value='Test9' />
+        <condition attribute='firstname' operator='ne' value='Test10' />
+        <condition attribute='firstname' operator='ne' value='Test11' />
+        <condition attribute='firstname' operator='ne' value='Test12' />
+        <condition attribute='firstname' operator='ne' value='Test13' />
+        <condition attribute='firstname' operator='ne' value='Test14' />
+        <condition attribute='firstname' operator='ne' value='Test15' />
+        <condition attribute='firstname' operator='ne' value='Test16' />
+        <condition attribute='firstname' operator='ne' value='Test17' />
+        <condition attribute='firstname' operator='ne' value='Test18' />
+        <condition attribute='firstname' operator='ne' value='Test19' />
+        <condition attribute='firstname' operator='ne' value='Test20' />
+        <condition attribute='firstname' operator='ne' value='Test21' />
+        <condition attribute='firstname' operator='ne' value='Test22' />
+        <condition attribute='firstname' operator='ne' value='Test23' />
+        <condition attribute='firstname' operator='ne' value='Test24' />
+        <condition attribute='firstname' operator='ne' value='Test25' />
+        <condition attribute='firstname' operator='ne' value='Test26' />
+        <condition attribute='firstname' operator='ne' value='Test27' />
+        <condition attribute='firstname' operator='ne' value='Test28' />
+        <condition attribute='firstname' operator='ne' value='Test29' />
+        <condition attribute='firstname' operator='ne' value='Test30' />
+        <condition attribute='firstname' operator='ne' value='Test31' />
+        <condition attribute='firstname' operator='ne' value='Test32' />
+        <condition attribute='firstname' operator='ne' value='Test33' />
+        <condition attribute='firstname' operator='ne' value='Test34' />
+        <condition attribute='firstname' operator='ne' value='Test35' />
+        <condition attribute='firstname' operator='ne' value='Test36' />
+        <condition attribute='firstname' operator='ne' value='Test37' />
+        <condition attribute='firstname' operator='ne' value='Test38' />
+        <condition attribute='firstname' operator='ne' value='Test39' />
+        <condition attribute='firstname' operator='ne' value='Test40' />
+        <condition attribute='firstname' operator='ne' value='Test41' />
+        <condition attribute='firstname' operator='ne' value='Test42' />
+        <condition attribute='firstname' operator='ne' value='Test43' />
+        <condition attribute='firstname' operator='ne' value='Test44' />
+        <condition attribute='firstname' operator='ne' value='Test45' />
+        <condition attribute='firstname' operator='ne' value='Test46' />
+        <condition attribute='firstname' operator='ne' value='Test47' />
+        <condition attribute='firstname' operator='ne' value='Test48' />
+        <condition attribute='firstname' operator='ne' value='Test49' />
+    </filter>
+  </entity>
 </fetch>");
         }
     }
diff --git a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
index f9b93320..7982f89e 100644
--- a/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
+++ b/MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
@@ -1729,6 +1729,44 @@ private void NormalizeFilters(NodeCompilationContext context)
             MergeRootFilters();
             MergeSingleConditionFilters();
             MergeNestedFilters();
+            RemoveDuplicatedConditions();
+        }
+
+        private void RemoveDuplicatedConditions()
+        {
+            // If we've got two copies of a condition in the same filter, remove one of them
+            Entity.Items = RemoveDuplicatedConditions(Entity.Items);
+        }
+
+        private object[] RemoveDuplicatedConditions(object[] items)
+        {
+            if (items == null)
+                return items;
+
+            var newItems = new List<object>();
+
+            foreach (var item in items)
+            {
+                if (item is condition c)
+                {
+                    if (newItems.OfType<condition>().Any(existing => existing.Equals(c)))
+                        continue;
+                }
+
+                if (item is filter f)
+                {
+                    f.Items = RemoveDuplicatedConditions(f.Items);
+                }
+
+                if (item is FetchLinkEntityType linkEntity)
+                {
+                    linkEntity.Items = RemoveDuplicatedConditions(linkEntity.Items);
+                }
+
+                newItems.Add(item);
+            }
+
+            return newItems.ToArray();
         }
 
         private void RemoveIdentitySemiJoinLinkEntities(NodeCompilationContext context)
@@ -1797,9 +1835,9 @@ private object[] MoveFiltersToLinkEntities(Dictionary<string, FetchLinkEntityTyp
 
             foreach (var filter in items.OfType<filter>().ToList())
             {
-                var entityName = GetConsistentEntityName(filter);
-
-                if (entityName != null && innerLinkEntities.TryGetValue(entityName, out var linkEntity))
+                if (IsConsistentEntityName(filter, out var entityName) &&
+                    entityName != null &&
+                    innerLinkEntities.TryGetValue(entityName, out var linkEntity))
                 {
                     linkEntity.AddItem(filter);
 
@@ -1826,24 +1864,43 @@ private void RemoveEntityName(filter filter)
                 RemoveEntityName(childFilter);
         }
 
-        private string GetConsistentEntityName(filter filter)
+        private bool IsConsistentEntityName(filter filter, out string entityName)
         {
-            var entityNames = filter.Items
-                .OfType<condition>()
-                .Select(c => c.entityname)
-                .Union(filter.Items.OfType<filter>().Select(GetConsistentEntityName))
-                .ToList();
+            if (filter.Items == null)
+            {
+                entityName = null;
+                return true;
+            }
 
-            if (entityNames.Count != 1)
-                return null;
+            entityName = null;
+            var setEntityName = false;
 
-            foreach (var childFilter in filter.Items.OfType<filter>())
+            foreach (var item in filter.Items)
             {
-                if (GetConsistentEntityName(childFilter) != entityNames[0])
-                    return null;
+                string currentEntityName;
+
+                if (item is condition c)
+                {
+                    currentEntityName = c.entityname;
+                }
+                else if (item is filter f)
+                {
+                    if (!IsConsistentEntityName(f, out currentEntityName))
+                        return false;
+                }
+                else
+                {
+                    continue;
+                }
+
+                if (setEntityName && entityName != currentEntityName)
+                    return false;
+
+                entityName = currentEntityName;
+                setEntityName = true;
             }
 
-            return entityNames[0];
+            return true;
         }
 
         private object[] MoveConditionsToLinkEntities(Dictionary<string, FetchLinkEntityType> innerLinkEntities, object[] items)
diff --git a/MarkMpn.Sql4Cds.Engine/FetchXml.Extensions.cs b/MarkMpn.Sql4Cds.Engine/FetchXml.Extensions.cs
index c19c2b77..9ec44897 100644
--- a/MarkMpn.Sql4Cds.Engine/FetchXml.Extensions.cs
+++ b/MarkMpn.Sql4Cds.Engine/FetchXml.Extensions.cs
@@ -14,6 +14,72 @@ partial class condition
         [XmlAttribute(Namespace = "MarkMpn.SQL4CDS")]
         [DefaultValue(false)]
         public bool IsVariable { get; set; }
+
+        public override bool Equals(object obj)
+        {
+            if (!(obj is condition other))
+                return false;
+
+            if (other.entityname != entityname ||
+                other.attribute != attribute ||
+                other.@operator != @operator ||
+                other.value != value ||
+                other.ValueOf != ValueOf ||
+                other.IsVariable != IsVariable ||
+                other.aggregate != aggregate ||
+                other.aggregateSpecified != aggregateSpecified ||
+                other.alias != alias ||
+                other.column != column ||
+                other.rowaggregate != rowaggregate ||
+                other.rowaggregateSpecified != rowaggregateSpecified)
+                return false;
+
+            if (other.Items == null ^ Items == null)
+                return false;
+
+            if (Items != null)
+            {
+                if (other.Items.Length != Items.Length)
+                    return false;
+
+                for (var i = 0; i < Items.Length; i++)
+                {
+                    if (other.Items[i].Value != Items[i].Value ||
+                        other.Items[i].IsVariable != Items[i].IsVariable)
+                        return false;
+                }
+            }
+
+            return true;
+        }
+
+        public override int GetHashCode()
+        {
+            var hash = 0;
+            hash ^= entityname?.GetHashCode() ?? 0;
+            hash ^= attribute?.GetHashCode() ?? 0;
+            hash ^= @operator.GetHashCode();
+            hash ^= value?.GetHashCode() ?? 0;
+            hash ^= ValueOf?.GetHashCode() ?? 0;
+            hash ^= IsVariable.GetHashCode();
+            hash ^= aggregate.GetHashCode();
+            hash ^= aggregateSpecified.GetHashCode();
+            hash ^= alias?.GetHashCode() ?? 0;
+            hash ^= column?.GetHashCode() ?? 0;
+            hash ^= rowaggregate.GetHashCode();
+            hash ^= rowaggregateSpecified.GetHashCode();
+
+            if (Items != null)
+            {
+                foreach (var item in Items)
+                {
+                    hash ^= item.Value?.GetHashCode() ?? 0;
+                    hash ^= item.IsVariable.GetHashCode();
+                }
+            }
+
+            return hash;
+        }
     }
 
     partial class conditionValue