You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@atlas.apache.org by sh...@apache.org on 2016/07/15 17:49:29 UTC

[2/2] incubator-atlas git commit: ATLAS-996 DSL queries with comparsions of many primitive types fail (jnhagelb via shwethags)

ATLAS-996 DSL queries with comparsions of many primitive types fail (jnhagelb via shwethags)


Project: http://git-wip-us.apache.org/repos/asf/incubator-atlas/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-atlas/commit/e895819f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-atlas/tree/e895819f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-atlas/diff/e895819f

Branch: refs/heads/master
Commit: e895819fec853d86ed5f8a4ee7fb7a7f1bd5aa1a
Parents: 1b1f9d2
Author: Shwetha GS <ss...@hortonworks.com>
Authored: Fri Jul 15 19:45:05 2016 +0530
Committer: Shwetha GS <ss...@hortonworks.com>
Committed: Fri Jul 15 19:45:05 2016 +0530

----------------------------------------------------------------------
 release-log.txt                                 |   1 +
 .../graph/GraphBackedSearchIndexer.java         |   1 +
 .../org/apache/atlas/query/Expressions.scala    |  14 +-
 .../org/apache/atlas/query/GremlinQuery.scala   |  76 ++++++----
 .../test/java/org/apache/atlas/TestUtils.java   |  43 +++++-
 .../GraphBackedDiscoveryServiceTest.java        | 142 +++++++++++++++----
 6 files changed, 218 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/e895819f/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index ecdc8a9..a18b9b1 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -6,6 +6,7 @@ INCOMPATIBLE CHANGES:
 
 
 ALL CHANGES:
+ATLAS-996 DSL queries with comparsions of many primitive types fail (jnhagelb via shwethags)
 ATLAS-971 UI not displaying results for this query - Eg: "hive_table as t where qualifiedName = 'default.input@cl1' select t" (kevalbhatt18 via shwethags) 
 ATLAS-1010 Atlas allows recreation of tags with same name (shwethags)
 ATLAS-990 Hive Import metadata script fails with auth exception (nixonrodrigues via shwethags)

http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/e895819f/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
----------------------------------------------------------------------
diff --git a/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java b/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
index 1a35d50..8c8134f 100755
--- a/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
+++ b/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
@@ -90,6 +90,7 @@ public class GraphBackedSearchIndexer implements SearchIndexer, ActiveStateChang
         try {
             if (management.containsPropertyKey(Constants.VERTEX_TYPE_PROPERTY_KEY)) {
                 LOG.info("Global indexes already exist for graph");
+                management.commit();
                 return;
             }
 

http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/e895819f/repository/src/main/scala/org/apache/atlas/query/Expressions.scala
----------------------------------------------------------------------
diff --git a/repository/src/main/scala/org/apache/atlas/query/Expressions.scala b/repository/src/main/scala/org/apache/atlas/query/Expressions.scala
index 2405750..ab7e81c 100755
--- a/repository/src/main/scala/org/apache/atlas/query/Expressions.scala
+++ b/repository/src/main/scala/org/apache/atlas/query/Expressions.scala
@@ -330,9 +330,9 @@ object Expressions {
         def instance() = new InstanceExpression(this)
 
         def path() = new PathExpression(this)
-        
+
         def limit(lmt: Literal[Integer], offset : Literal[Integer]) = new LimitExpression(this, lmt, offset)
-        
+
         def order(odr: String, asc: Boolean) = new OrderExpression(this, odr, asc)
     }
 
@@ -635,7 +635,11 @@ object Expressions {
                 left.dataType;
             } else if(left.dataType == DataTypes.DATE_TYPE) {
                 DataTypes.DATE_TYPE
-            } else if (left.dataType != DataTypes.STRING_TYPE || right.dataType != DataTypes.STRING_TYPE) {
+            }
+            else if(left.dataType == DataTypes.BOOLEAN_TYPE) {
+                DataTypes.BOOLEAN_TYPE;
+            }
+            else if (left.dataType != DataTypes.STRING_TYPE || right.dataType != DataTypes.STRING_TYPE) {
                 TypeUtils.combinedType(left.dataType, right.dataType)
             }
             DataTypes.BOOLEAN_TYPE
@@ -783,8 +787,8 @@ object Expressions {
             child.dataType
     }
   }
-  
-  case class OrderExpression(child: Expression, odr: String, asc: Boolean) extends Expression with UnaryNode { 
+
+  case class OrderExpression(child: Expression, odr: String, asc: Boolean) extends Expression with UnaryNode {
 
     override def toString = s"$child  order $odr asc $asc"
 

http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/e895819f/repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala
----------------------------------------------------------------------
diff --git a/repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala b/repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala
index 14c42b0..8add6c8 100755
--- a/repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala
+++ b/repository/src/main/scala/org/apache/atlas/query/GremlinQuery.scala
@@ -18,6 +18,7 @@
 
 package org.apache.atlas.query
 
+import org.apache.atlas.query.TypeUtils.FieldInfo;
 import org.apache.atlas.query.Expressions._
 import org.apache.atlas.typesystem.types.{TypeSystem, DataTypes}
 import org.apache.atlas.typesystem.types.DataTypes.TypeCategory
@@ -25,6 +26,7 @@ import org.joda.time.format.ISODateTimeFormat
 
 import scala.collection.mutable
 import scala.collection.mutable.ArrayBuffer
+import org.apache.atlas.typesystem.types.IDataType
 
 trait IntSequence {
     def next: Int
@@ -237,30 +239,9 @@ class GremlinTranslator(expr: Expression,
             }
         }
         case c@ComparisonExpression(symb, f@FieldExpression(fieldName, fInfo, ch), l) => {
-          val QUOTE = "\"";
-          val fieldGremlinExpr = s"${gPersistenceBehavior.fieldNameInVertex(fInfo.dataType, fInfo.attrInfo)}"
-            ch match {
-                case Some(child) => {
-                  s"""${genQuery(child, inSelect)}.has("$fieldGremlinExpr", ${gPersistenceBehavior.gremlinCompOp(c)}, $l)"""
-                }
-                case None => {
-                    if (fInfo.attrInfo.dataType == DataTypes.DATE_TYPE) {
-                        try {
-                            //Accepts both date, datetime formats
-                            val dateStr = l.toString.stripPrefix(QUOTE).stripSuffix(QUOTE)
-                            val dateVal = ISODateTimeFormat.dateOptionalTimeParser().parseDateTime(dateStr).getMillis
-                            s"""has("$fieldGremlinExpr", ${gPersistenceBehavior.gremlinCompOp(c)},${dateVal})"""
-                        } catch {
-                            case pe: java.text.ParseException =>
-                                throw new GremlinTranslationException(c,
-                                    "Date format " + l + " not supported. Should be of the format " + TypeSystem.getInstance().getDateFormat.toPattern);
-
-                        }
-                    }
-                    else
-                        s"""has("$fieldGremlinExpr", ${gPersistenceBehavior.gremlinCompOp(c)}, $l)"""
-                }
-            }
+           val qualifiedPropertyName = s"${gPersistenceBehavior.fieldNameInVertex(fInfo.dataType, fInfo.attrInfo)}";
+           val persistentExprValue = translateValueToPersistentForm(fInfo, l);
+           return generateAndPrependExpr(ch, inSelect, s"""has("${qualifiedPropertyName}", ${gPersistenceBehavior.gremlinCompOp(c)}, $persistentExprValue)""");
         }
         case fil@FilterExpression(child, condExpr) => {
             s"${genQuery(child, inSelect)}.${genQuery(condExpr, inSelect)}"
@@ -344,6 +325,53 @@ class GremlinTranslator(expr: Expression,
         case x => throw new GremlinTranslationException(x, "expression not yet supported")
     }
 
+
+    def translateValueToPersistentForm(fInfo: FieldInfo, l: Expression): Any =  {
+
+        val dataType = fInfo.attrInfo.dataType;
+
+         if (dataType == DataTypes.DATE_TYPE) {
+              val QUOTE = "\"";
+              try {
+                  //Accepts both date, datetime formats
+                  val dateStr = l.toString.stripPrefix(QUOTE).stripSuffix(QUOTE)
+                  val dateVal = ISODateTimeFormat.dateOptionalTimeParser().parseDateTime(dateStr).getMillis
+                  return dateVal
+               } catch {
+                     case pe: java.text.ParseException =>
+                          throw new GremlinTranslationException(l,
+                                      "Date format " + l + " not supported. Should be of the format " +
+                                      TypeSystem.getInstance().getDateFormat.toPattern);
+               }
+         }
+         else if(dataType == DataTypes.BYTE_TYPE) {
+             //cast needed, otherwise get class cast exception when trying to compare, since the
+             //persist value is assumed to be an Integer
+             return s"""(byte)$l"""
+         }
+         else if(dataType == DataTypes.SHORT_TYPE) {
+             return s"""(short)$l"""
+         }
+         else if(dataType == DataTypes.LONG_TYPE) {
+             return s"""${l}L"""
+         }
+         else if(dataType == DataTypes.FLOAT_TYPE) {
+             return s"""${l}f"""
+         }
+         else if(dataType == DataTypes.DOUBLE_TYPE) {
+             return s"""${l}d"""
+         }
+         else {
+             return l
+         }
+    }
+
+    def generateAndPrependExpr(e1: Option[Expression], inSelect: Boolean, e2: String) : String = e1 match {
+
+        case Some(x) => s"""${genQuery(x, inSelect)}.$e2"""
+        case None => e2
+    }
+
     def genFullQuery(expr: Expression): String = {
         var q = genQuery(expr, false)
         if(gPersistenceBehavior.addGraphVertexPrefix(preStatements)) {

http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/e895819f/repository/src/test/java/org/apache/atlas/TestUtils.java
----------------------------------------------------------------------
diff --git a/repository/src/test/java/org/apache/atlas/TestUtils.java b/repository/src/test/java/org/apache/atlas/TestUtils.java
index 1eeffb4..bd9df62 100755
--- a/repository/src/test/java/org/apache/atlas/TestUtils.java
+++ b/repository/src/test/java/org/apache/atlas/TestUtils.java
@@ -46,6 +46,8 @@ import org.codehaus.jettison.json.JSONArray;
 import org.testng.Assert;
 
 import java.io.File;
+import java.math.BigDecimal;
+import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
@@ -113,7 +115,18 @@ public final class TestUtils {
                 createOptionalAttrDef("address", "Address"),
                 new AttributeDefinition("department", "Department", Multiplicity.REQUIRED, false, "employees"),
                 new AttributeDefinition("manager", "Manager", Multiplicity.OPTIONAL, false, "subordinates"),
-                new AttributeDefinition("mentor", "Person", Multiplicity.OPTIONAL, false, null));
+                new AttributeDefinition("mentor", "Person", Multiplicity.OPTIONAL, false, null),
+                createOptionalAttrDef("birthday", DataTypes.DATE_TYPE),
+                createOptionalAttrDef("hasPets", DataTypes.BOOLEAN_TYPE),
+                createOptionalAttrDef("numberOfCars", DataTypes.BYTE_TYPE),
+                createOptionalAttrDef("houseNumber", DataTypes.SHORT_TYPE),
+                createOptionalAttrDef("carMileage", DataTypes.INT_TYPE),
+                createOptionalAttrDef("shares", DataTypes.LONG_TYPE),
+                createOptionalAttrDef("salary", DataTypes.DOUBLE_TYPE),
+                createOptionalAttrDef("age", DataTypes.FLOAT_TYPE),
+                createOptionalAttrDef("numberOfStarsEstimate", DataTypes.BIGINTEGER_TYPE),
+                createOptionalAttrDef("approximationOfPi", DataTypes.BIGDECIMAL_TYPE)
+                );
 
         HierarchicalTypeDefinition<ClassType> managerTypeDef = createClassTypeDef("Manager", "Manager"+_description, ImmutableSet.of("Person"),
                 new AttributeDefinition("subordinates", String.format("array<%s>", "Person"), Multiplicity.COLLECTION,
@@ -142,7 +155,7 @@ public final class TestUtils {
         Referenceable juliusAddr = new Referenceable("Address");
         Referenceable max = new Referenceable("Person");
         Referenceable maxAddr = new Referenceable("Address");
-        
+
         hrDept.set("name", "hr");
         john.set("name", "John");
         john.set("department", hrDept);
@@ -150,11 +163,23 @@ public final class TestUtils {
         johnAddr.set("city", "Sunnyvale");
         john.set("address", johnAddr);
 
+        john.set("birthday",new Date(1950, 5, 15));
+        john.set("hasPets", true);
+        john.set("numberOfCars", 1);
+        john.set("houseNumber", 153);
+        john.set("carMileage", 13364);
+        john.set("shares", 15000);
+        john.set("salary", 123345.678);
+        john.set("age", 50);
+        john.set("numberOfStarsEstimate", new BigInteger("1000000000000000000000"));
+        john.set("approximationOfPi", new BigDecimal("3.141592653589793238462643383279502884197169399375105820974944592307816406286"));
+
         jane.set("name", "Jane");
         jane.set("department", hrDept);
         janeAddr.set("street", "Great America Parkway");
         janeAddr.set("city", "Santa Clara");
         jane.set("address", janeAddr);
+        janeAddr.set("street", "Great America Parkway");
 
         julius.set("name", "Julius");
         julius.set("department", hrDept);
@@ -162,7 +187,7 @@ public final class TestUtils {
         juliusAddr.set("city", "Newtonville");
         julius.set("address", juliusAddr);
         julius.set("subordinates", ImmutableList.<Referenceable>of());
-        
+
         max.set("name", "Max");
         max.set("department", hrDept);
         maxAddr.set("street", "Ripley St");
@@ -170,7 +195,17 @@ public final class TestUtils {
         max.set("address", maxAddr);
         max.set("manager", jane);
         max.set("mentor", julius);
-        
+        max.set("birthday",new Date(1979, 3, 15));
+        max.set("hasPets", true);
+        max.set("age", 36);
+        max.set("numberOfCars", 2);
+        max.set("houseNumber", 17);
+        max.set("carMileage", 13);
+        max.set("shares", Long.MAX_VALUE);
+        max.set("salary", Double.MAX_VALUE);
+        max.set("numberOfStarsEstimate", new BigInteger("1000000000000000000000000000000"));
+        max.set("approximationOfPi", new BigDecimal("3.1415926535897932"));
+
         john.set("manager", jane);
         john.set("mentor", max);
         hrDept.set("employees", ImmutableList.of(john, jane, julius, max));

http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/e895819f/repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java
----------------------------------------------------------------------
diff --git a/repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java b/repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java
index a911c49..c27894e 100755
--- a/repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java
+++ b/repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java
@@ -19,7 +19,7 @@
 package org.apache.atlas.discovery;
 
 import com.google.common.collect.ImmutableSet;
-
+import org.apache.atlas.AtlasException;
 import org.apache.atlas.BaseRepositoryTest;
 import org.apache.atlas.RepositoryMetadataModule;
 import org.apache.atlas.RequestContext;
@@ -27,12 +27,15 @@ import org.apache.atlas.TestUtils;
 import org.apache.atlas.discovery.graph.GraphBackedDiscoveryService;
 import org.apache.atlas.repository.Constants;
 import org.apache.atlas.repository.MetadataRepository;
+import org.apache.atlas.repository.graph.GraphBackedSearchIndexer;
+import org.apache.atlas.repository.graph.TitanGraphProvider;
 import org.apache.atlas.typesystem.ITypedReferenceableInstance;
 import org.apache.atlas.typesystem.Referenceable;
 import org.apache.atlas.typesystem.persistence.Id;
 import org.apache.atlas.typesystem.types.ClassType;
 import org.apache.atlas.typesystem.types.DataTypes;
 import org.apache.atlas.typesystem.types.HierarchicalTypeDefinition;
+import org.apache.atlas.typesystem.types.IDataType;
 import org.apache.atlas.typesystem.types.Multiplicity;
 import org.apache.atlas.typesystem.types.TypeSystem;
 import org.codehaus.jettison.json.JSONArray;
@@ -46,12 +49,14 @@ import org.testng.annotations.Guice;
 import org.testng.annotations.Test;
 
 import javax.inject.Inject;
-
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static org.apache.atlas.typesystem.types.utils.TypesUtil.createClassTypeDef;
 import static org.apache.atlas.typesystem.types.utils.TypesUtil.createOptionalAttrDef;
@@ -71,20 +76,48 @@ public class GraphBackedDiscoveryServiceTest extends BaseRepositoryTest {
     @BeforeClass
     public void setUp() throws Exception {
         super.setUp();
-        TypeSystem typeSystem = TypeSystem.getInstance();
+
+        final TypeSystem typeSystem = TypeSystem.getInstance();
+        Collection<String> oldTypeNames = new HashSet<String>();
+        oldTypeNames.addAll(typeSystem.getTypeNames());
+
         TestUtils.defineDeptEmployeeTypes(typeSystem);
 
+        addIndexesForNewTypes(oldTypeNames, typeSystem);
+
         ITypedReferenceableInstance hrDept = TestUtils.createDeptEg1(typeSystem);
         repositoryService.createEntities(hrDept);
-        
+
         ITypedReferenceableInstance jane = repositoryService.getEntityDefinition("Manager", "name", "Jane");
         Id janeGuid = jane.getId();
         ClassType personType = typeSystem.getDataType(ClassType.class, "Person");
         ITypedReferenceableInstance instance = personType.createInstance(janeGuid);
         instance.set("orgLevel", "L1");
-        repositoryService.updateEntities(instance);
+        repositoryService.updatePartial(instance);
     }
 
+    private void addIndexesForNewTypes(Collection<String> oldTypeNames, final TypeSystem typeSystem) throws AtlasException {
+        Set<String> newTypeNames = new HashSet<>();
+        newTypeNames.addAll(typeSystem.getTypeNames());
+        newTypeNames.removeAll(oldTypeNames);
+
+        Collection<IDataType> newTypes = new ArrayList<IDataType>();
+        for(String name : newTypeNames) {
+            try {
+                newTypes.add(typeSystem.getDataType(IDataType.class, name));
+            } catch (AtlasException e) {
+                e.printStackTrace();
+            }
+
+        }
+        TitanGraphProvider provider = new TitanGraphProvider();
+        //We need to commit the transaction before creating the indices to release the locks held by the transaction.
+        //otherwise, the index commit will fail while waiting for the those locks to be released.
+        provider.get().commit();
+        GraphBackedSearchIndexer idx = new GraphBackedSearchIndexer(provider);
+        idx.onAdd(newTypes);
+	}
+
     @BeforeMethod
     public void setupContext() {
         RequestContext.createContext();
@@ -207,6 +240,73 @@ public class GraphBackedDiscoveryServiceTest extends BaseRepositoryTest {
         }
     }
 
+    @DataProvider(name = "comparisonQueriesProvider")
+    private Object[][] createComparisonQueries() {
+        //create queries the exercise the comparison logic for
+        //all of the different supported data types
+        return new Object[][] {
+            {"Person where (birthday < \"1950-01-01T02:35:58.440Z\" )", 0},
+            {"Person where (birthday > \"1975-01-01T02:35:58.440Z\" )", 2},
+            {"Person where (birthday >= \"1975-01-01T02:35:58.440Z\" )", 2},
+            {"Person where (birthday <= \"1950-01-01T02:35:58.440Z\" )", 0},
+            {"Person where (birthday = \"1975-01-01T02:35:58.440Z\" )", 0},
+            {"Person where (birthday != \"1975-01-01T02:35:58.440Z\" )", 4},
+
+            {"Person where (hasPets = true)", 2},
+            {"Person where (hasPets = false)", 2},
+            {"Person where (hasPets != false)", 2},
+            {"Person where (hasPets != true)", 2},
+
+            {"Person where (numberOfCars > 0)", 2},
+            {"Person where (numberOfCars > 1)", 1},
+            {"Person where (numberOfCars >= 1)", 2},
+            {"Person where (numberOfCars < 2)", 3},
+            {"Person where (numberOfCars <= 2)", 4},
+            {"Person where (numberOfCars = 2)", 1},
+            {"Person where (numberOfCars != 2)", 3},
+
+            {"Person where (houseNumber > 0)", 2},
+            {"Person where (houseNumber > 17)", 1},
+            {"Person where (houseNumber >= 17)", 2},
+            {"Person where (houseNumber < 153)", 3},
+            {"Person where (houseNumber <= 153)", 4},
+            {"Person where (houseNumber =  17)", 1},
+            {"Person where (houseNumber != 17)", 3},
+
+            {"Person where (carMileage > 0)", 2},
+            {"Person where (carMileage > 13)", 1},
+            {"Person where (carMileage >= 13)", 2},
+            {"Person where (carMileage < 13364)", 3},
+            {"Person where (carMileage <= 13364)", 4},
+            {"Person where (carMileage =  13)", 1},
+            {"Person where (carMileage != 13)", 3},
+
+            {"Person where (shares > 0)", 2},
+            {"Person where (shares > 13)", 2},
+            {"Person where (shares >= 16000)", 1},
+            {"Person where (shares < 13364)", 2},
+            {"Person where (shares <= 15000)", 3},
+            {"Person where (shares =  15000)", 1},
+            {"Person where (shares != 1)", 4},
+
+            {"Person where (salary > 0)", 2},
+            {"Person where (salary > 100000)", 2},
+            {"Person where (salary >= 200000)", 1},
+            {"Person where (salary < 13364)", 2},
+            {"Person where (salary <= 150000)", 3},
+            {"Person where (salary =  12334)", 0},
+            {"Person where (salary != 12344)", 4},
+
+            {"Person where (age > 36)", 1},
+            {"Person where (age > 49)", 1},
+            {"Person where (age >= 49)", 1},
+            {"Person where (age < 50)", 3},
+            {"Person where (age <= 35)", 2},
+            {"Person where (age =  35)", 0},
+            {"Person where (age != 35)", 4}
+        };
+    }
+
     @DataProvider(name = "dslQueriesProvider")
     private Object[][] createDSLQueries() {
         return new Object[][]{
@@ -577,6 +677,15 @@ public class GraphBackedDiscoveryServiceTest extends BaseRepositoryTest {
     
     @Test(dataProvider = "dslQueriesProvider")
     public void  testSearchByDSLQueries(String dslQuery, Integer expectedNumRows) throws Exception {
+        runQuery(dslQuery, expectedNumRows);
+    }
+
+    @Test(dataProvider = "comparisonQueriesProvider")
+    public void testDataTypeComparisonQueries(String dslQuery, Integer expectedNumRows) throws Exception {
+        runQuery(dslQuery, expectedNumRows);
+    }
+
+    public void runQuery(String dslQuery, Integer expectedNumRows) throws Exception {
         System.out.println("Executing dslQuery = " + dslQuery);
         String jsonResults = discoveryService.searchByDSL(dslQuery);
         assertNotNull(jsonResults);
@@ -595,32 +704,13 @@ public class GraphBackedDiscoveryServiceTest extends BaseRepositoryTest {
 
         JSONArray rows = results.getJSONArray("rows");
         assertNotNull(rows);
-        assertEquals(rows.length(), expectedNumRows.intValue()); // some queries may not have any results
+        assertEquals( rows.length(), expectedNumRows.intValue(), "query [" + dslQuery + "] returned [" + rows.length() + "] rows.  Expected " + expectedNumRows.intValue() + " rows."); // some queries may not have any results
         System.out.println("query [" + dslQuery + "] returned [" + rows.length() + "] rows");
     }
 
     @Test(dataProvider = "dslLimitQueriesProvider")
     public void  testSearchByDSLQueriesWithLimit(String dslQuery, Integer expectedNumRows) throws Exception {
-        System.out.println("Executing dslQuery = " + dslQuery);
-        String jsonResults = discoveryService.searchByDSL(dslQuery);
-        assertNotNull(jsonResults);
-
-        JSONObject results = new JSONObject(jsonResults);
-        assertEquals(results.length(), 3);
-        System.out.println("results = " + results);
-
-        Object query = results.get("query");
-        assertNotNull(query);
-
-        JSONObject dataType = results.getJSONObject("dataType");
-        assertNotNull(dataType);
-        String typeName = dataType.getString("typeName");
-        assertNotNull(typeName);
-
-        JSONArray rows = results.getJSONArray("rows");
-        assertNotNull(rows);
-        assertEquals(rows.length(), expectedNumRows.intValue()); // some queries may not have any results
-        System.out.println("query [" + dslQuery + "] returned [" + rows.length() + "] rows");
+        runQuery(dslQuery, expectedNumRows);
     }
     
     @DataProvider(name = "invalidDslQueriesProvider")