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 <[email protected]> Authored: Fri Jul 15 19:45:05 2016 +0530 Committer: Shwetha GS <[email protected]> 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")
