From 5bb189f477f6a2e5a9b0f228f3b0103269146bea Mon Sep 17 00:00:00 2001
From: lukaseder <lukas.eder@gmail.com>
Date: Tue, 24 Sep 2013 22:31:46 +0200
Subject: [PATCH] [#2580] Bad SQL rendered when combining DISTINCT with LIMIT
 .. OFFSET in DB2, SQL Server

---
 .../jooq/test/_/testcases/OrderByTests.java   | 26 +++++++++++++++++++
 .../src/org/jooq/test/jOOQAbstractTest.java   |  5 ++++
 .../java/org/jooq/impl/SelectQueryImpl.java   | 20 +++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/jOOQ-test/src/org/jooq/test/_/testcases/OrderByTests.java b/jOOQ-test/src/org/jooq/test/_/testcases/OrderByTests.java
index a54244fc8e..9bbd26790a 100644
--- a/jOOQ-test/src/org/jooq/test/_/testcases/OrderByTests.java
+++ b/jOOQ-test/src/org/jooq/test/_/testcases/OrderByTests.java
@@ -307,6 +307,32 @@ extends BaseTest<A, AP, B, S, B2S, BS, L, X, DATE, BOOL, D, T, U, UU, I, IPK, T7
         assertEquals(Integer.valueOf(3), result.getValue(1, TBook_ID()));
     }
 
+    @Test
+    public void testLimitDistinct() throws Exception {
+        assertEquals(asList(1, 2),
+        create().selectDistinct(TBookToBookStore_BOOK_ID())
+                .from(TBookToBookStore())
+                .orderBy(TBookToBookStore_BOOK_ID())
+                .limit(2)
+                .fetch(TBookToBookStore_BOOK_ID()));
+
+        assertEquals(asList(2, 3),
+        create().selectDistinct(TBookToBookStore_BOOK_ID())
+                .from(TBookToBookStore())
+                .orderBy(TBookToBookStore_BOOK_ID())
+                .limit(2)
+                .offset(1)
+                .fetch(TBookToBookStore_BOOK_ID()));
+
+        assertSame(asList(2, 1),
+        create().selectDistinct(TBook_LANGUAGE_ID(), TBook_AUTHOR_ID())
+                .from(TBook())
+                .orderBy(TBook_AUTHOR_ID().desc())
+                .limit(2)
+                .offset(1)
+                .fetch(TBook_AUTHOR_ID()));
+    }
+
     @Test
     public void testLimitAliased() throws Exception {
         if (asList(ASE, SQLSERVER).contains(dialect().family())) {
diff --git a/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java b/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java
index c2cb511266..9c08827fb9 100644
--- a/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java
+++ b/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java
@@ -1804,6 +1804,11 @@ public abstract class jOOQAbstractTest<
         new OrderByTests(this).testLimit();
     }
 
+    @Test
+    public void testLimitDistinct() throws Exception {
+        new OrderByTests(this).testLimitDistinct();
+    }
+
     @Test
     public void testLimitAliased() throws Exception {
         new OrderByTests(this).testLimitAliased();
diff --git a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java
index b636120d32..c9dc9acdd3 100644
--- a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java
+++ b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java
@@ -61,6 +61,7 @@ import static org.jooq.SQLDialect.SQLSERVER;
 import static org.jooq.SQLDialect.SQLSERVER2008;
 import static org.jooq.SQLDialect.SQLSERVER2012;
 import static org.jooq.conf.ParamType.INLINED;
+import static org.jooq.impl.DSL.denseRank;
 import static org.jooq.impl.DSL.inline;
 import static org.jooq.impl.DSL.name;
 import static org.jooq.impl.DSL.one;
@@ -68,6 +69,7 @@ import static org.jooq.impl.DSL.rowNumber;
 import static org.jooq.impl.Utils.DATA_ROW_VALUE_EXPRESSION_PREDICATE_SUBQUERY;
 import static org.jooq.impl.Utils.DATA_WRAP_DERIVED_TABLES_IN_PARENTHESES;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
@@ -381,7 +383,23 @@ class SelectQueryImpl<R extends Record> extends AbstractSelect<R> implements Sel
         // window function, calculating row numbers for the LIMIT .. OFFSET clause
         RenderContext local = new DefaultRenderContext(context);
         local.subquery(true);
-        toSQLReference0(local, rowNumber().over().orderBy(getNonEmptyOrderBy()).as(rownumName));
+
+        // [#2580] When DISTINCT is applied, we mustn't use ROW_NUMBER() OVER(),
+        // which changes the DISTINCT semantics. Instead, use DENSE_RANK() OVER(),
+        // ordering by the SELECT's ORDER BY clause AND all the expressions from
+        // the projection
+        if (distinct) {
+            List<SortField<?>> order = new ArrayList<SortField<?>>();
+            order.addAll(getNonEmptyOrderBy());
+
+            // TODO: Challenge this with lots of additional tests, improve readability
+            for (Field<?> field : getSelect())
+                order.add(field.asc());
+            toSQLReference0(local, denseRank().over().orderBy(order).as(rownumName));
+        }
+        else {
+            toSQLReference0(local, rowNumber().over().orderBy(getNonEmptyOrderBy()).as(rownumName));
+        }
         String enclosed = local.render();
 
         context.keyword("select * from (")