You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2017/08/01 21:03:13 UTC

[19/50] [abbrv] geode git commit: GEODE-2936: Refactoring OrderByComparator and updating OrderByComparatorJUnitTest

GEODE-2936: Refactoring OrderByComparator and updating OrderByComparatorJUnitTest

This closes #580


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/06b44db8
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/06b44db8
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/06b44db8

Branch: refs/heads/feature/GEODE-3299
Commit: 06b44db8ccf9016899ae79c663147a0829293a94
Parents: 6267809
Author: Jinmei Liao <ji...@pivotal.io>
Authored: Wed Jun 7 16:10:56 2017 -0700
Committer: Ken Howe <kh...@pivotal.io>
Committed: Wed Jul 26 15:27:08 2017 -0700

----------------------------------------------------------------------
 .../query/internal/CompiledSortCriterion.java   |   6 +-
 .../cache/query/internal/OrderByComparator.java | 208 ++++++++-----------
 .../internal/OrderByComparatorUnmapped.java     |   6 +-
 .../query/functional/StructSetOrResultsSet.java |  13 +-
 .../internal/OrderByComparatorJUnitTest.java    |  69 +++---
 5 files changed, 134 insertions(+), 168 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/06b44db8/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSortCriterion.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSortCriterion.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSortCriterion.java
index d49b4a5..4d2be62 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSortCriterion.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSortCriterion.java
@@ -56,7 +56,7 @@ public class CompiledSortCriterion extends AbstractCompiledValue {
    * evaluates sort criteria in order by clause
    */
   public Object evaluate(Object data, ExecutionContext context) {
-    Object value = null;
+    Object value;
     if (this.columnIndex > 0) {
       value = ((Object[]) data)[this.columnIndex];
     } else if (this.columnIndex == 0) {
@@ -113,7 +113,7 @@ public class CompiledSortCriterion extends AbstractCompiledValue {
   }
 
   private CompiledValue getReconstructedExpression(String projAttribStr, ExecutionContext context)
-      throws AmbiguousNameException, TypeMismatchException, NameResolutionException {
+      throws TypeMismatchException, NameResolutionException {
     List<CompiledValue> expressions = PathUtils.collectCompiledValuesInThePath(expr, context);
     StringBuilder tempBuff = new StringBuilder();
     ListIterator<CompiledValue> listIter = expressions.listIterator(expressions.size());
@@ -180,7 +180,7 @@ public class CompiledSortCriterion extends AbstractCompiledValue {
   }
 
   boolean mapExpressionToProjectionField(List projAttrs, ExecutionContext context)
-      throws AmbiguousNameException, TypeMismatchException, NameResolutionException {
+      throws TypeMismatchException, NameResolutionException {
     boolean mappedColumn = false;
     this.originalCorrectedExpression = expr;
     if (projAttrs != null) {

http://git-wip-us.apache.org/repos/asf/geode/blob/06b44db8/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
index 59eb493..04469cb 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java
@@ -15,7 +15,6 @@
 package org.apache.geode.cache.query.internal;
 
 import java.util.Comparator;
-import java.util.Iterator;
 import java.util.List;
 
 import org.apache.geode.cache.query.FunctionDomainException;
@@ -28,9 +27,8 @@ import org.apache.geode.internal.cache.VMCachedDeserializable;
 import org.apache.geode.pdx.internal.PdxString;
 
 /**
- * A generic comparator class which compares two Object/StructImpl according to their sort criterion
- * specified in order by clause
- * 
+ * A generic comparator class which compares two Object/StructImpl according to their sort criteria
+ * specified in order by clause.
  */
 public class OrderByComparator implements Comparator {
   private final ObjectType objType;
@@ -45,78 +43,44 @@ public class OrderByComparator implements Comparator {
   }
 
   /**
-   * Yogesh : This methods evaluates sort criteria and returns a ArrayList of Object[] arrays of
-   * evaluated criteria
+   * This method evaluates sort criteria and returns an ArrayList of Object[] arrays of the
+   * evaluated criteria.
    * 
-   * @param value
-   * @return Object[]
+   * @param value the criteria to be evaluated.
+   * @return an Object array of Object arrays of the evaluated criteria.
    */
   protected Object[] evaluateSortCriteria(Object value) {
-
-    CompiledSortCriterion csc;
     Object[] array = null;
     if (orderByAttrs != null) {
       array = new Object[orderByAttrs.size()];
-      Iterator orderiter = orderByAttrs.iterator();
       int i = 0;
-      while (orderiter.hasNext()) {
-        csc = (CompiledSortCriterion) orderiter.next();
-        Object[] arr = new Object[2];
-        arr[0] = csc.evaluate(value, context);
-        arr[1] = Boolean.valueOf(csc.getCriterion());
+      for (CompiledSortCriterion csc : orderByAttrs) {
+        Object[] arr = {csc.evaluate(value, context), csc.getCriterion()};
         array[i++] = arr;
       }
-
     }
     return array;
   }
 
+  /**
+   * This method evaluates sort criteria and returns the resulting integer value of comparing the
+   * two objects passed into it based on these criteria.
+   *
+   * @param value1 the first object to be compared.
+   * @param value2 the second object to be compared.
+   * @return a negative integer, zero, or a positive integer if the first argument is less than,
+   *         equal to, or greater than the second, based on the evaluated sort criteria.
+   */
   protected int evaluateSortCriteria(Object value1, Object value2) {
     int result = -1;
-    CompiledSortCriterion csc;
     if (orderByAttrs != null) {
-      Iterator orderiter = orderByAttrs.iterator();
-      while (orderiter.hasNext()) {
-        csc = (CompiledSortCriterion) orderiter.next();
+      for (CompiledSortCriterion csc : orderByAttrs) {
         Object sortCriteriaForValue1 = csc.evaluate(value1, context);
         Object sortCriteriaForValue2 = csc.evaluate(value2, context);
-
-        if (sortCriteriaForValue1 == null || sortCriteriaForValue2 == null) {
-          if (sortCriteriaForValue1 == null) {
-            result = (sortCriteriaForValue2 == null ? 0 : -1);
-          } else {
-            result = 1;
-          }
-        } else if (sortCriteriaForValue1 == QueryService.UNDEFINED
-            || sortCriteriaForValue2 == QueryService.UNDEFINED) {
-          if (sortCriteriaForValue1 == QueryService.UNDEFINED) {
-            result = (sortCriteriaForValue2 == QueryService.UNDEFINED ? 0 : -1);
-          } else {
-            result = 1;
-          }
-        } else {
-          if (sortCriteriaForValue1 instanceof Number && sortCriteriaForValue2 instanceof Number) {
-            double diff = ((Number) sortCriteriaForValue1).doubleValue()
-                - ((Number) sortCriteriaForValue2).doubleValue();
-            result = diff > 0 ? 1 : diff < 0 ? -1 : 0;
-          } else {
-            if (sortCriteriaForValue1 instanceof PdxString
-                && sortCriteriaForValue2 instanceof String) {
-              sortCriteriaForValue2 = new PdxString((String) sortCriteriaForValue2);
-            } else if (sortCriteriaForValue2 instanceof PdxString
-                && sortCriteriaForValue1 instanceof String) {
-              sortCriteriaForValue1 = new PdxString((String) sortCriteriaForValue1);
-            }
-            result = ((Comparable) sortCriteriaForValue1).compareTo(sortCriteriaForValue2);
-          }
-
-        }
-
-        if (result == 0) {
-          continue;
-        } else {
-          if (Boolean.valueOf(csc.getCriterion())) {
-            result = (result * (-1));
+        result = compareHelperMethod(sortCriteriaForValue1, sortCriteriaForValue2);
+        if (result != 0) {
+          if (csc.getCriterion()) {
+            result *= -1;
           }
           break;
         }
@@ -137,89 +101,35 @@ public class OrderByComparator implements Comparator {
    *         Comparator.
    */
   public int compare(Object obj1, Object obj2) {
-    int result = -1;
+    int result;
     if (obj1 == null && obj2 == null) {
       return 0;
     }
     assert !(obj1 instanceof VMCachedDeserializable || obj2 instanceof VMCachedDeserializable);
-
     if ((this.objType.isStructType() && obj1 instanceof Object[] && obj2 instanceof Object[])
-        || !this.objType.isStructType()) { // obj1 instanceof Object && obj2
-                                           // instanceof Object){
-      if ((result = evaluateSortCriteria(obj1, obj2)) != 0) {
+        || !this.objType.isStructType()) {
+      if (((result = evaluateSortCriteria(obj1, obj2)) != 0) && (orderByAttrs != null)) {
         return result;
       }
-
-      QueryObserver observer = QueryObserverHolder.getInstance();
-      if (observer != null) {
-        observer.orderByColumnsEqual();
+      if (QueryObserverHolder.getInstance() != null) {
+        QueryObserverHolder.getInstance().orderByColumnsEqual();
       }
-      // The comparable fields are equal, so we check if the overall keys are
-      // equal or not
+      // Comparable fields are equal - check if overall keys are equal
       if (this.objType.isStructType()) {
         int i = 0;
         for (Object o1 : (Object[]) obj1) {
           Object o2 = ((Object[]) obj2)[i++];
-
-          // Check for null value.
-          if (o1 == null || o2 == null) {
-            if (o1 == null) {
-              if (o2 == null) {
-                continue;
-              }
-              return -1;
-            } else {
-              return 1;
-            }
-          } else if (o1 == QueryService.UNDEFINED || o2 == QueryService.UNDEFINED) {
-            if (o1 == QueryService.UNDEFINED) {
-              if (o2 == QueryService.UNDEFINED) {
-                continue;
-              }
-              return -1;
-            } else {
-              return 1;
-            }
-          }
-
-          if (o1 instanceof Comparable) {
-            final int rslt;
-            if (o1 instanceof Number && o2 instanceof Number) {
-              double diff = ((Number) o1).doubleValue() - ((Number) o2).doubleValue();
-              rslt = diff > 0 ? 1 : diff < 0 ? -1 : 0;
-            } else {
-              if (o1 instanceof PdxString && o2 instanceof String) {
-                o2 = new PdxString((String) o2);
-              } else if (o2 instanceof PdxString && o1 instanceof String) {
-                o1 = new PdxString((String) o1);
-              }
-              rslt = ((Comparable) o1).compareTo(o2);
-            }
-            if (rslt == 0) {
-              continue;
-            } else {
-              return rslt;
-            }
-          } else if (!o1.equals(o2)) {
-            return -1;
+          result = compareHelperMethod(o1, o2);
+          if (result != 0) {
+            return result;
           }
         }
         return 0;
       } else {
-        if (obj1 instanceof PdxString && obj2 instanceof String) {
-          obj2 = new PdxString((String) obj2);
-        } else if (obj2 instanceof PdxString && obj1 instanceof String) {
-          obj1 = new PdxString((String) obj1);
-        }
-
-        if (obj1 instanceof Comparable) {
-          return ((Comparable) obj1).compareTo(obj2);
-        } else {
-          return obj1.equals(obj2) ? 0 : -1;
-        }
+        return compareTwoStrings(obj1, obj2);
       }
     }
-    return -1;
+    throw new ClassCastException(); // throw exception if args can't be compared w/this comparator
   }
 
   void addEvaluatedSortCriteria(Object row, ExecutionContext context)
@@ -228,4 +138,56 @@ public class OrderByComparator implements Comparator {
     // No op
   }
 
+  private int compareHelperMethod(Object obj1, Object obj2) {
+    if (obj1 == null || obj2 == null) {
+      return compareIfOneOrMoreNull(obj1, obj2);
+    } else if (obj1 == QueryService.UNDEFINED || obj2 == QueryService.UNDEFINED) {
+      return compareIfOneOrMoreQueryServiceUndefined(obj1, obj2);
+    } else {
+      return compareTwoObjects(obj1, obj2);
+    }
+  }
+
+  private int compareIfOneOrMoreNull(Object obj1, Object obj2) {
+    if (obj1 == null) {
+      return obj2 == null ? 0 : -1;
+    } else {
+      return 1;
+    }
+  }
+
+  private int compareIfOneOrMoreQueryServiceUndefined(Object obj1, Object obj2) {
+    if (obj1 == QueryService.UNDEFINED) {
+      return obj2 == QueryService.UNDEFINED ? 0 : -1;
+    } else {
+      return 1;
+    }
+  }
+
+  private int compareTwoObjects(Object obj1, Object obj2) {
+    if (obj1 instanceof Number && obj2 instanceof Number) {
+      return compareTwoNumbers(obj1, obj2);
+    } else {
+      return compareTwoStrings(obj1, obj2);
+    }
+  }
+
+  private int compareTwoNumbers(Object obj1, Object obj2) {
+    Number num1 = (Number) obj1;
+    Number num2 = (Number) obj2;
+    return Double.compare(num1.doubleValue(), num2.doubleValue());
+  }
+
+  private int compareTwoStrings(Object obj1, Object obj2) {
+    if (obj1 instanceof PdxString && obj2 instanceof String) {
+      obj2 = new PdxString((String) obj2);
+    } else if (obj2 instanceof PdxString && obj1 instanceof String) {
+      obj1 = new PdxString((String) obj1);
+    }
+    if (obj1 instanceof Comparable) {
+      return ((Comparable) obj1).compareTo(obj2);
+    } else {
+      return obj1.equals(obj2) ? 0 : -1;
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/06b44db8/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparatorUnmapped.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparatorUnmapped.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparatorUnmapped.java
index db31df9..ee2caa0 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparatorUnmapped.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparatorUnmapped.java
@@ -14,13 +14,13 @@
  */
 package org.apache.geode.cache.query.internal;
 
-import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap;
-
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap;
+
 import org.apache.geode.cache.query.FunctionDomainException;
 import org.apache.geode.cache.query.NameResolutionException;
 import org.apache.geode.cache.query.QueryInvocationTargetException;
@@ -113,7 +113,7 @@ public class OrderByComparatorUnmapped extends OrderByComparator {
 
   @Override
   protected Object[] evaluateSortCriteria(Object row) {
-    return (Object[]) orderByMap.get(row);
+    return orderByMap.get(row);
   }
 
 

http://git-wip-us.apache.org/repos/asf/geode/blob/06b44db8/geode-core/src/test/java/org/apache/geode/cache/query/functional/StructSetOrResultsSet.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/functional/StructSetOrResultsSet.java b/geode-core/src/test/java/org/apache/geode/cache/query/functional/StructSetOrResultsSet.java
index fa5b3cd..b328cfe 100755
--- a/geode-core/src/test/java/org/apache/geode/cache/query/functional/StructSetOrResultsSet.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/functional/StructSetOrResultsSet.java
@@ -18,7 +18,9 @@
  */
 package org.apache.geode.cache.query.functional;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
@@ -93,8 +95,8 @@ public class StructSetOrResultsSet {
   public void CompareQueryResultsWithoutAndWithIndexes(Object[][] r, int len, boolean checkOrder,
       String queries[]) {
 
-    Collection coll1 = null;
-    Collection coll2 = null;
+    Collection coll1;
+    Collection coll2;
     for (int j = 0; j < len; j++) {
       checkSelectResultTypes((SelectResults) r[j][0], (SelectResults) r[j][1], queries[j]);
       checkResultSizes((SelectResults) r[j][0], (SelectResults) r[j][1], queries[j]);
@@ -113,7 +115,7 @@ public class StructSetOrResultsSet {
   public void compareExternallySortedQueriesWithOrderBy(String[] queries, Object[][] baseResults)
       throws Exception {
     for (int i = 0; i < queries.length; i++) {
-      Query q = null;
+      Query q;
       try {
         String query = queries[i];
         int indexOfOrderBy = query.indexOf("order ");
@@ -124,7 +126,7 @@ public class StructSetOrResultsSet {
         int unorderedResultsSize = ((SelectResults) baseResults[i][1]).size();
         if (unorderedResultsSize == 0) {
           fail(
-              "The test results size is 0 , it possibly is not validating anything. rewrite the test");
+              "The test results size is 0. It may not be validating anything. Please rewrite the test.");
         }
         Wrapper wrapper = getOrderByComparatorAndLimitForQuery(queries[i], unorderedResultsSize);
         if (wrapper.validationLevel != ValidationLevel.NONE) {
@@ -230,7 +232,6 @@ public class StructSetOrResultsSet {
         @Override
         public int compare(Struct o1, Struct o2) {
           return obc.compare(o1.getFieldValues(), o2.getFieldValues());
-
         }
       };
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/06b44db8/geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
index 4d5174e..6c365cd 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java
@@ -14,12 +14,15 @@
  */
 package org.apache.geode.cache.query.internal;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.lang.reflect.Field;
 import java.util.Collection;
+import java.util.List;
 
 import org.junit.After;
 import org.junit.Before;
@@ -32,9 +35,9 @@ import org.apache.geode.cache.Region;
 import org.apache.geode.cache.query.CacheUtils;
 import org.apache.geode.cache.query.Query;
 import org.apache.geode.cache.query.QueryInvalidException;
-import org.apache.geode.cache.query.QueryService;
 import org.apache.geode.cache.query.data.Portfolio;
 import org.apache.geode.cache.query.data.Position;
+import org.apache.geode.cache.query.internal.types.StructTypeImpl;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
@@ -43,7 +46,6 @@ public class OrderByComparatorJUnitTest {
   @Before
   public void setUp() throws java.lang.Exception {
     CacheUtils.startCache();
-
   }
 
   @After
@@ -53,18 +55,13 @@ public class OrderByComparatorJUnitTest {
 
   @Test
   public void testOrderByComparatorUnmapped() throws Exception {
-
     String queries[] = {
-
         "SELECT  distinct ID, description, createTime FROM /portfolio1 pf1 where ID > 0 order by ID desc, pkid desc ",};
     Object r[][] = new Object[queries.length][2];
-    QueryService qs;
-    qs = CacheUtils.getQueryService();
     Position.resetCounter();
-    // Create Regions
 
+    // Create Regions
     Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
-
     for (int i = 0; i < 10; i++) {
       r1.put(i + "", new Portfolio(i));
     }
@@ -85,7 +82,6 @@ public class OrderByComparatorJUnitTest {
         assertTrue(base instanceof SortedStructSet);
         SortedStructSet sss = (SortedStructSet) base;
         assertTrue(sss.comparator() instanceof OrderByComparatorUnmapped);
-
       } catch (Exception e) {
         e.printStackTrace();
         fail(q.getQueryString());
@@ -95,16 +91,12 @@ public class OrderByComparatorJUnitTest {
 
   @Test
   public void testOrderByComparatorMapped() throws Exception {
-
     String queries[] = {
-
         "SELECT  distinct ID, description, createTime, pkid FROM /portfolio1 pf1 where ID > 0 order by ID desc, pkid desc ",};
     Object r[][] = new Object[queries.length][2];
-    QueryService qs;
-    qs = CacheUtils.getQueryService();
     Position.resetCounter();
-    // Create Regions
 
+    // Create Regions
     Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
 
     for (int i = 0; i < 10; i++) {
@@ -128,7 +120,6 @@ public class OrderByComparatorJUnitTest {
         SortedStructSet sss = (SortedStructSet) base;
         assertFalse(sss.comparator() instanceof OrderByComparatorUnmapped);
         assertTrue(sss.comparator() instanceof OrderByComparator);
-
       } catch (Exception e) {
         e.printStackTrace();
         fail(q.getQueryString());
@@ -138,14 +129,9 @@ public class OrderByComparatorJUnitTest {
 
   @Test
   public void testUnsupportedOrderByForPR() throws Exception {
-
     String unsupportedQueries[] =
-        {"select distinct p.status from /portfolio1 p order by p.status, p.ID",
-
-        };
+        {"select distinct p.status from /portfolio1 p order by p.status, p.ID"};
     Object r[][] = new Object[unsupportedQueries.length][2];
-    QueryService qs;
-    qs = CacheUtils.getQueryService();
     Position.resetCounter();
     // Create Regions
     PartitionAttributesFactory paf = new PartitionAttributesFactory();
@@ -158,8 +144,7 @@ public class OrderByComparatorJUnitTest {
     }
 
     for (int i = 0; i < unsupportedQueries.length; i++) {
-      Query q = null;
-
+      Query q;
       CacheUtils.getLogger().info("Executing query: " + unsupportedQueries[i]);
       q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
       try {
@@ -173,17 +158,12 @@ public class OrderByComparatorJUnitTest {
 
   @Test
   public void testSupportedOrderByForRR() throws Exception {
-
     String unsupportedQueries[] =
-        {"select distinct p.status from /portfolio1 p order by p.status, p.ID",
-
-        };
+        {"select distinct p.status from /portfolio1 p order by p.status, p.ID"};
     Object r[][] = new Object[unsupportedQueries.length][2];
-    QueryService qs;
-    qs = CacheUtils.getQueryService();
     Position.resetCounter();
-    // Create Regions
 
+    // Create Regions
     Region r1 = CacheUtils.createRegion("portfolio1", Portfolio.class);
 
     for (int i = 0; i < 50; i++) {
@@ -191,13 +171,11 @@ public class OrderByComparatorJUnitTest {
     }
 
     for (int i = 0; i < unsupportedQueries.length; i++) {
-      Query q = null;
-
+      Query q;
       CacheUtils.getLogger().info("Executing query: " + unsupportedQueries[i]);
       q = CacheUtils.getQueryService().newQuery(unsupportedQueries[i]);
       try {
         r[i][0] = q.execute();
-
       } catch (QueryInvalidException qe) {
         qe.printStackTrace();
         fail(qe.toString());
@@ -205,4 +183,29 @@ public class OrderByComparatorJUnitTest {
     }
   }
 
+  // The following tests cover edge cases in OrderByComparator.
+  @Test
+  public void testCompareTwoNulls() throws Exception {
+    assertThat(createComparator().compare(null, null)).isEqualTo(0);
+  }
+
+  @Test
+  public void testCompareTwoObjectArrays() throws Exception {
+    String[] arrString1 = {"elephants"};
+    String[] arrString2 = {"elephant"};
+    assertThat(createComparator().compare(arrString1, arrString2)).isEqualTo(1);
+  }
+
+  @Test
+  public void testCompareThrowsClassCastException() throws Exception {
+    String testString = "elephant";
+    int testInt = 159;
+    assertThatThrownBy(() -> createComparator().compare(testString, testInt))
+        .isInstanceOf(ClassCastException.class);
+  }
+
+  private OrderByComparator createComparator() throws Exception {
+    StructTypeImpl objType = new StructTypeImpl();
+    return new OrderByComparator(null, objType, null);
+  }
 }