#747 - @History findVersionsBetween query with Oracle has incorrect ordering

src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java

@@ -19,6 +19,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.sql.SQLException;
+import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -204,15 +205,16 @@ public class CQueryEngine {
 
     SpiQuery<T> query = request.getQuery();
 
+    String sysPeriodLower = getSysPeriodLower(query);
     if (query.isVersionsBetween() && !historySupport.isBindAtFromClause()) {
       // just add as normal predicates using the lower bound
-      query.where().gt(getSysPeriodLower(query), query.getVersionStart());
-      query.where().lt(getSysPeriodLower(query), query.getVersionEnd());
+      query.where().gt(sysPeriodLower, query.getVersionStart());
+      query.where().lt(sysPeriodLower, query.getVersionEnd());
     }
 
     // order by id asc, lower sys period desc
     query.orderBy().asc(request.getBeanDescriptor().getIdProperty().getName());
-    query.orderBy().desc(getSysPeriodLower(query));
+    query.orderBy().desc(sysPeriodLower);
 
     CQuery<T> cquery = queryBuilder.buildQuery(request);
     try {
@@ -222,6 +224,9 @@ public class CQueryEngine {
       }
 
       List<Version<T>> versions = cquery.readVersions();
+      // just order in memory rather than use NULLS LAST as that
+      // is not universally supported, not expect huge list here
+      Collections.sort(versions, OrderVersionDesc.INSTANCE);
       deriveVersionDiffs(versions, request);
 
       if (request.isLogSummary()) {

src/main/java/com/avaje/ebeaninternal/server/query/OrderVersionDesc.java

@@ -0,0 +1,29 @@
+package com.avaje.ebeaninternal.server.query;
+
+import com.avaje.ebean.Version;
+
+import java.sql.Timestamp;
+import java.util.Comparator;
+
+/**
+ * Compare Version beans in descending order with nulls last.
+ */
+class OrderVersionDesc implements Comparator<Version<?>> {
+
+  static final OrderVersionDesc INSTANCE = new OrderVersionDesc();
+
+  @Override
+  public int compare(Version<?> o1, Version<?> o2) {
+
+    Timestamp v1 = o1.getStart();
+    if (v1 == null) {
+      return 1;
+    }
+    Timestamp v2 = o2.getStart();
+    if (v2 == null) {
+      return -1;
+    }
+    return v1.compareTo(v2) * -1;
+  }
+
+}

src/test/java/com/avaje/ebeaninternal/server/query/OrderVersionDescTest.java

@@ -0,0 +1,81 @@
+package com.avaje.ebeaninternal.server.query;
+
+import com.avaje.ebean.Version;
+import org.junit.Test;
+
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+
+public class OrderVersionDescTest {
+
+  private final long now = System.currentTimeMillis();
+
+  @Test
+  public void sort() {
+
+    Version<?> atNull = atNull();
+    Version<?> at100 = at(100);
+    Version<?> at200 = at(200);
+    Version<?> at300 = at(300);
+
+    List<Version<?>> versions = new ArrayList<Version<?>>();
+    versions.add(at200);
+    versions.add(atNull);
+    versions.add(at300);
+    versions.add(at100);
+
+    Collections.sort(versions, OrderVersionDesc.INSTANCE);
+
+    assertThat(versions.get(0)).isSameAs(at300);
+    assertThat(versions.get(1)).isSameAs(at200);
+    assertThat(versions.get(2)).isSameAs(at100);
+    assertThat(versions.get(3)).isSameAs(atNull);
+  }
+
+  @Test
+  public void compare_lt() {
+
+    assertEquals(OrderVersionDesc.INSTANCE.compare(at(0), at(1)), 1);
+  }
+
+  @Test
+  public void compare_gt() {
+
+    assertEquals(OrderVersionDesc.INSTANCE.compare(at(2), at(1)), -1);
+  }
+
+  @Test
+  public void compare_eq() {
+
+    assertEquals(OrderVersionDesc.INSTANCE.compare(at(1), at(1)), 0);
+  }
+
+  @Test
+  public void compare_nullFirst() {
+
+    assertEquals(OrderVersionDesc.INSTANCE.compare(atNull(), at(1)), 1);
+  }
+
+
+  @Test
+  public void compare_nullLast() {
+
+    assertEquals(OrderVersionDesc.INSTANCE.compare(at(0), atNull()), -1);
+  }
+
+  private Version<?> atNull() {
+    return new Version();
+  }
+
+  private Version at(long diff) {
+    Timestamp timestamp = new Timestamp(now +  diff);
+    Version ver = new Version();
+    ver.setStart(timestamp);
+    return ver;
+  }
+}