Implemented proper, join-aware paging in SQL Server

markrendle · Nov 6, 2012 · 54581ae · 54581ae
1 parent 5bcbf2e
commit 54581ae
Show file tree

Hide file tree

Showing 11 changed files with 216 additions and 172 deletions.
diff --git a/Simple.Data.Ado.Test/ProviderHelperTest.cs b/Simple.Data.Ado.Test/ProviderHelperTest.cs
@@ -1,118 +1,118 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using NUnit.Framework;
-using System.Data;
-using Simple.Data.Ado.Schema;
-using System.ComponentModel.Composition;
-
-namespace Simple.Data.Ado.Test
-{
-    [TestFixture]
-    public class ProviderHelperTest
-    {
-        [Test]
-        public void ShouldNotRequestExportableTypeFromServiceProvider()
-        {
-            var helper = new ProviderHelper();
-            var connectionProvider = new StubConnectionAndServiceProvider();
-            var actual = helper.GetCustomProvider<ITestInterface>(connectionProvider);
-            Assert.IsNull(connectionProvider.RequestedServiceType);
-        }
-
-        [Test]
-        public void ShouldRequestNonExportedTypeFromServiceProvider()
-        {
-            var helper = new ProviderHelper();
-            var connectionProvider = new StubConnectionAndServiceProvider();
-            var actual = helper.GetCustomProvider<IQueryPager>(connectionProvider);
-            Assert.AreEqual(typeof(IQueryPager), connectionProvider.RequestedServiceType);
-        }
-
-        [Test]
-        public void ShouldReturnNonExportedTypeFromServiceProvider()
-        {
-            var helper = new ProviderHelper();
-            var connectionProvider = new StubConnectionAndServiceProvider();
-            var actual = helper.GetCustomProvider<IQueryPager>(connectionProvider);
-            Assert.IsInstanceOf(typeof(IQueryPager), actual);
-        }
-
-        [Test]
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using NUnit.Framework;
+using System.Data;
+using Simple.Data.Ado.Schema;
+using System.ComponentModel.Composition;
+
+namespace Simple.Data.Ado.Test
+{
+    [TestFixture]
+    public class ProviderHelperTest
+    {
+        [Test]
+        public void ShouldNotRequestExportableTypeFromServiceProvider()
+        {
+            var helper = new ProviderHelper();
+            var connectionProvider = new StubConnectionAndServiceProvider();
+            var actual = helper.GetCustomProvider<ITestInterface>(connectionProvider);
+            Assert.IsNull(connectionProvider.RequestedServiceType);
+        }
+
+        [Test]
+        public void ShouldRequestNonExportedTypeFromServiceProvider()
+        {
+            var helper = new ProviderHelper();
+            var connectionProvider = new StubConnectionAndServiceProvider();
+            var actual = helper.GetCustomProvider<IQueryPager>(connectionProvider);
+            Assert.AreEqual(typeof(IQueryPager), connectionProvider.RequestedServiceType);
+        }
+
+        [Test]
+        public void ShouldReturnNonExportedTypeFromServiceProvider()
+        {
+            var helper = new ProviderHelper();
+            var connectionProvider = new StubConnectionAndServiceProvider();
+            var actual = helper.GetCustomProvider<IQueryPager>(connectionProvider);
+            Assert.IsInstanceOf(typeof(IQueryPager), actual);
+        }
+
+        [Test]
         public void ShouldFindProviderUsingAssemblyAttribute()
         {
             IConnectionProvider provider;
             Assert.True(ProviderHelper.TryLoadAssemblyUsingAttribute("Test", null, out provider));
             Assert.IsNotNull(provider);
             Assert.IsInstanceOf<StubConnectionProvider>(provider);
-        }
-
-        public class StubConnectionAndServiceProvider : IConnectionProvider, IServiceProvider
-        {
-            public void SetConnectionString(string connectionString)
-            {
-                throw new NotImplementedException();
-            }
-
-            public IDbConnection CreateConnection()
-            {
-                throw new NotImplementedException();
-            }
-
-            public ISchemaProvider GetSchemaProvider()
-            {
-                throw new NotImplementedException();
-            }
-
-            public string ConnectionString
-            {
-                get { throw new NotImplementedException(); }
-            }
-
-            public bool SupportsCompoundStatements
-            {
-                get { throw new NotImplementedException(); }
-            }
-
-            public string GetIdentityFunction()
-            {
-                throw new NotImplementedException();
-            }
-
-            public bool SupportsStoredProcedures
-            {
-                get { throw new NotImplementedException(); }
-            }
-
-            public IProcedureExecutor GetProcedureExecutor(AdoAdapter adapter, ObjectName procedureName)
-            {
-                throw new NotImplementedException();
-            }
-
-            public Type RequestedServiceType { get; private set; }
-            public Object GetService(Type serviceType)
-            {
-                this.RequestedServiceType = serviceType;
-                return new StubQueryPager();
-            }
-        }
-
-        public class StubQueryPager : IQueryPager
+        }
+
+        public class StubConnectionAndServiceProvider : IConnectionProvider, IServiceProvider
+        {
+            public void SetConnectionString(string connectionString)
+            {
+                throw new NotImplementedException();
+            }
+
+            public IDbConnection CreateConnection()
+            {
+                throw new NotImplementedException();
+            }
+
+            public ISchemaProvider GetSchemaProvider()
+            {
+                throw new NotImplementedException();
+            }
+
+            public string ConnectionString
+            {
+                get { throw new NotImplementedException(); }
+            }
+
+            public bool SupportsCompoundStatements
+            {
+                get { throw new NotImplementedException(); }
+            }
+
+            public string GetIdentityFunction()
+            {
+                throw new NotImplementedException();
+            }
+
+            public bool SupportsStoredProcedures
+            {
+                get { throw new NotImplementedException(); }
+            }
+
+            public IProcedureExecutor GetProcedureExecutor(AdoAdapter adapter, ObjectName procedureName)
+            {
+                throw new NotImplementedException();
+            }
+
+            public Type RequestedServiceType { get; private set; }
+            public Object GetService(Type serviceType)
+            {
+                this.RequestedServiceType = serviceType;
+                return new StubQueryPager();
+            }
+        }
+
+        public class StubQueryPager : IQueryPager
         {
             public IEnumerable<string> ApplyLimit(string sql, int take)
             {
                 throw new NotImplementedException();
             }
 
-            public IEnumerable<string> ApplyPaging(string sql, int skip, int take)
-            {
-                throw new NotImplementedException();
-            }
-        }
-
-        public interface ITestInterface { }
-        [Export(typeof(ITestInterface))]
-        public class TestClass : ITestInterface { }
-    }
-}
+            public IEnumerable<string> ApplyPaging(string sql, string[] keys, int skip, int take)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        public interface ITestInterface { }
+        [Export(typeof(ITestInterface))]
+        public class TestClass : ITestInterface { }
+    }
+}
diff --git a/Simple.Data.Ado/AdoAdapterQueryRunner.cs b/Simple.Data.Ado/AdoAdapterQueryRunner.cs
@@ -151,7 +151,7 @@ public AdoAdapterQueryRunner(AdoAdapter adapter, AdoAdapterTransaction transacti
                 }
                 else
                 {
-                    ApplyPaging(commandBuilders, mainCommandBuilder, skipClause, takeClause, query.Clauses.OfType<WithClause>().Any(), queryPager);
+                    ApplyPaging(query, commandBuilders, mainCommandBuilder, skipClause, takeClause, query.Clauses.OfType<WithClause>().Any(), queryPager);
                 }
             }
             return commandBuilders.ToArray();
@@ -168,7 +168,7 @@ public AdoAdapterQueryRunner(AdoAdapter adapter, AdoAdapterTransaction transacti
             commandBuilders.Add(commandBuilder);
         }
 
-        private void ApplyPaging(List<ICommandBuilder> commandBuilders, ICommandBuilder mainCommandBuilder, SkipClause skipClause, TakeClause takeClause, bool hasWithClause, IQueryPager queryPager)
+        private void ApplyPaging(SimpleQuery query, List<ICommandBuilder> commandBuilders, ICommandBuilder mainCommandBuilder, SkipClause skipClause, TakeClause takeClause, bool hasWithClause, IQueryPager queryPager)
         {
             const int maxInt = 2147483646;
 
@@ -179,9 +179,17 @@ private void ApplyPaging(List<ICommandBuilder> commandBuilders, ICommandBuilder
             }
             else
             {
+                var table = _adapter.GetSchema().FindTable(query.TableName);
+                if (table.PrimaryKey == null || table.PrimaryKey.Length == 0)
+                {
+                    throw new AdoAdapterException("Cannot apply paging to a table with no primary key.");
+                }
+                var keys = table.PrimaryKey.AsEnumerable()
+                     .Select(k => string.Format("{0}.{1}", table.QualifiedName, _adapter.GetSchema().QuoteObjectName(k)))
+                     .ToArray();
                 int skip = skipClause == null ? 0 : skipClause.Count;
                 int take = takeClause == null ? maxInt : takeClause.Count;
-                commandTexts = queryPager.ApplyPaging(mainCommandBuilder.Text, skip, take);
+                commandTexts = queryPager.ApplyPaging(mainCommandBuilder.Text, keys,  skip, take);
             }
 
             commandBuilders.AddRange(

diff --git a/Simple.Data.Ado/IQueryPager.cs b/Simple.Data.Ado/IQueryPager.cs
@@ -5,6 +5,6 @@ namespace Simple.Data.Ado
     public interface IQueryPager
     {
         IEnumerable<string> ApplyLimit(string sql, int take);
-        IEnumerable<string> ApplyPaging(string sql, int skip, int take);
+        IEnumerable<string> ApplyPaging(string sql, string[] keys, int skip, int take);
     }
 }
diff --git a/Simple.Data.Ado/Joiner.cs b/Simple.Data.Ado/Joiner.cs
@@ -121,12 +121,12 @@ private string MakeJoinText(Table rightTable, string alias, ForeignKey foreignKe
             builder.AppendFormat(" JOIN {0}", rightTable.QualifiedName);
             if (!string.IsNullOrWhiteSpace(alias)) builder.Append(" " + _schema.QuoteObjectName(alias));
             builder.Append(" ON (");
-            builder.Append(FormatJoinExpression(foreignKey, 0, alias));
+            builder.Append(FormatJoinExpression(foreignKey, 0, rightTable, alias));
 
             for (int i = 1; i < foreignKey.Columns.Length; i++)
             {
                 builder.Append(" AND ");
-                builder.Append(FormatJoinExpression(foreignKey, i, alias));
+                builder.Append(FormatJoinExpression(foreignKey, i, rightTable, alias));
             }
             builder.Append(")");
             return builder.ToString();
@@ -167,13 +167,25 @@ private SimpleExpression CreateJoinExpression(ObjectReference table, ForeignKey
             return masterObjectReference == detailObjectReference;
         }
 
-        private string FormatJoinExpression(ForeignKey foreignKey, int columnIndex, string alias)
+        private string FormatJoinExpression(ForeignKey foreignKey, int columnIndex, Table rightTable, string alias)
         {
-            var leftTable = string.IsNullOrWhiteSpace(alias)
-                                ? _schema.QuoteObjectName(foreignKey.MasterTable)
-                                : _schema.QuoteObjectName(alias);
-            return string.Format("{0}.{1} = {2}.{3}", leftTable, _schema.QuoteObjectName(foreignKey.UniqueColumns[columnIndex]),
-                                 _schema.QuoteObjectName(foreignKey.DetailTable), _schema.QuoteObjectName(foreignKey.Columns[columnIndex]));
+            if (rightTable.ActualName == foreignKey.MasterTable.Name &&
+                rightTable.Schema == foreignKey.MasterTable.Schema)
+            {
+                var rightTableName = string.IsNullOrWhiteSpace(alias)
+                                    ? _schema.QuoteObjectName(foreignKey.MasterTable)
+                                    : _schema.QuoteObjectName(alias);
+                return string.Format("{0}.{1} = {2}.{3}",
+                    rightTableName, _schema.QuoteObjectName(foreignKey.UniqueColumns[columnIndex]),
+                    _schema.QuoteObjectName(foreignKey.DetailTable), _schema.QuoteObjectName(foreignKey.Columns[columnIndex])
+                    );
+            }
+
+            var leftTableName = string.IsNullOrWhiteSpace(alias)
+                                    ? _schema.QuoteObjectName(foreignKey.DetailTable)
+                                    : _schema.QuoteObjectName(alias);
+            return string.Format("{0}.{1} = {2}.{3}", _schema.QuoteObjectName(foreignKey.MasterTable), _schema.QuoteObjectName(foreignKey.UniqueColumns[columnIndex]),
+                                 leftTableName, _schema.QuoteObjectName(foreignKey.Columns[columnIndex]));
         }
 
         private string JoinKeyword

diff --git a/Simple.Data.SqlCe40/SqlCe40QueryPager.cs b/Simple.Data.SqlCe40/SqlCe40QueryPager.cs
@@ -19,7 +19,7 @@ public IEnumerable<string> ApplyLimit(string sql, int take)
             yield return SelectMatch.Replace(sql, match => match.Value + " TOP(" + take + ") ");
         }
 
-        public IEnumerable<string> ApplyPaging(string sql, int skip, int take)
+        public IEnumerable<string> ApplyPaging(string sql, string[] keys, int skip, int take)
         {
             if (sql.IndexOf("order by", StringComparison.InvariantCultureIgnoreCase) < 0)
             {

diff --git a/Simple.Data.SqlCe40Test/SqlCe40QueryPagerTest.cs b/Simple.Data.SqlCe40Test/SqlCe40QueryPagerTest.cs
@@ -29,7 +29,7 @@ public void ShouldApplyPagingUsingOrderBy()
             var expected = new[]{
                 "select a,b,c from d where a = 1 order by c offset 5 rows fetch next 10 rows only"};
 
-            var pagedSql = new SqlCe40QueryPager().ApplyPaging(sql, 5, 10);
+            var pagedSql = new SqlCe40QueryPager().ApplyPaging(sql, new string[0], 5, 10);
             var modified = pagedSql.Select(x=> Normalize.Replace(x, " ").ToLowerInvariant());
 
             Assert.IsTrue(expected.SequenceEqual(modified));
@@ -42,7 +42,7 @@ public void ShouldApplyPagingUsingOrderByFirstColumnIfNotAlreadyOrdered()
             var expected = new[]{
                 "select a,b,c from d where a = 1 order by a offset 10 rows fetch next 20 rows only"};
 
-            var pagedSql = new SqlCe40QueryPager().ApplyPaging(sql, 10, 20);
+            var pagedSql = new SqlCe40QueryPager().ApplyPaging(sql, new string[0], 10, 20);
             var modified = pagedSql.Select(x => Normalize.Replace(x, " ").ToLowerInvariant());
 
             Assert.IsTrue(expected.SequenceEqual(modified));