----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60159/#review178625 -----------------------------------------------------------
graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/TitanGraphQuery.java Lines 161 (patched) <https://reviews.apache.org/r/60159/#comment252732> if(LOG.isDebugEnabled()) { LOG.debug("Executing: " + queryCondition.toString()); } graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/TitanGraphQuery.java Lines 169 (patched) <https://reviews.apache.org/r/60159/#comment252737> should this loop break out when "result.size() == limit"? intg/src/main/java/org/apache/atlas/model/discovery/AtlasSearchResult.java Line 46 (original), 45 (patched) <https://reviews.apache.org/r/60159/#comment252739> 'searchParameters' duplicates the values in other attributes here: queryText, type, classification, entities, attributes. Is 'searchParameters' attribute needed? intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java Lines 138 (patched) <https://reviews.apache.org/r/60159/#comment252740> It will be useful to use an enum for 'operator'. repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java Lines 412 (patched) <https://reviews.apache.org/r/60159/#comment252742> Wouldn't skipping result entry here cause fewer number of entries in 'ret' - if searchPipeline.run() already restricted the result to 'searchParameters.limit' entries. This is better handled in searchPipeline.run(). repository/src/main/java/org/apache/atlas/discovery/GremlinSearchStep.java Lines 68 (patched) <https://reviews.apache.org/r/60159/#comment252743> if (LOG.isDebugEnabled()) { } repository/src/main/java/org/apache/atlas/discovery/GremlinSearchStep.java Lines 74 (patched) <https://reviews.apache.org/r/60159/#comment252747> Is 'typeName' mandatory in SearchParameters? repository/src/main/java/org/apache/atlas/discovery/GremlinSearchStep.java Lines 76 (patched) <https://reviews.apache.org/r/60159/#comment252744> add a warn level log here. repository/src/main/java/org/apache/atlas/discovery/GremlinSearchStep.java Lines 81 (patched) <https://reviews.apache.org/r/60159/#comment252745> Validate that the given classification exists, similar to entity type valuration above in lines #74 - #78. repository/src/main/java/org/apache/atlas/discovery/GremlinSearchStep.java Lines 89 (patched) <https://reviews.apache.org/r/60159/#comment252746> searchParameters.getTypeName() ==> searchParameters.getClassification(): attributes referenced in tagFilters will be defined in the classification (and not the given entity). Better yet, consider passing AtlasClassification object to toGremlinQuery method. This will avoid multiple lookups inside toGremlinQuery() - Madhan Neethiraj On June 22, 2017, 5:24 a.m., Apoorv Naik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60159/ > ----------------------------------------------------------- > > (Updated June 22, 2017, 5:24 a.m.) > > > Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath > Subramanian, and Suma Shivaprasad. > > > Bugs: ATLAS-1880 > https://issues.apache.org/jira/browse/ATLAS-1880 > > > Repository: atlas > > > Description > ------- > > See JIRA > > > Sample Request structure > > ```json > { > "typeName": "hive_table", > "excludeDeletedEntities": false, > "limit": 25, > "offset": 0, > "entityFilters": { > "condition": "AND", > "criterion": [{ > "attributeName": "retention", > "operator": "=", > "attributeValue": "10" > }, { > "condition": "OR", > "criterion": [{ > "attributeName": "createTime", > "operator": ">", > "attributeValue": "1111111111" > }, { > "attributeName": "lastAccessTime", > "operator": "<=", > "attributeValue": "2222222222" > }] > }] > } > } > ``` > > > Diffs > ----- > > client/src/main/java/org/apache/atlas/AtlasClientV2.java 61413428 > common/src/main/java/org/apache/atlas/repository/Constants.java ac022528 > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphQuery.java > 841edf71 > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasIndexQuery.java > 1ff9d5ed > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertexQuery.java > 53f490f5 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanGraphQuery.java > 0211ff05 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/TitanGraphQuery.java > 0077a21f > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/AndCondition.java > 68f0eb28 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/HasPredicate.java > 24e4f5bc > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0IndexQuery.java > 1ed1734f > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0VertexQuery.java > bd8b897f > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/NativeTitan0GraphQuery.java > 7ec6ffeb > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1IndexQuery.java > 4073dd2a > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1VertexQuery.java > 4452bcdd > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/NativeTitan1GraphQuery.java > 1ca900d8 > intg/src/main/java/org/apache/atlas/model/discovery/AtlasSearchResult.java > a402c628 > intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportRequest.java > b19f7097 > > repository/src/main/java/org/apache/atlas/discovery/AtlasDiscoveryService.java > 923a198b > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > 128cdbf9 > repository/src/main/java/org/apache/atlas/discovery/GremlinSearchStep.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/discovery/SearchPipeline.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/discovery/SolrSearchStep.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java > 4ffbb88c > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > 75e91320 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java > 00fe94b6 > repository/src/test/java/org/apache/atlas/TestModules.java d28956de > > repository/src/test/java/org/apache/atlas/discovery/GremlinSearchStepTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/services/EntityDiscoveryServiceTest.java > 5d5b043e > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > 8c5623fa > webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java ea550211 > webapp/src/main/webapp/WEB-INF/web.xml 9b5c3b14 > webapp/src/test/java/org/apache/atlas/web/resources/AdminResourceTest.java > 1fe31198 > > > Diff: https://reviews.apache.org/r/60159/diff/2/ > > > Testing > ------- > > 1. Tested Json marshalling and unmarshalling via REST > 2. Tested with the attached request JSONs > > In progress > > UTs (coding) > > > File Attachments > ---------------- > > Asset contains > > https://reviews.apache.org/media/uploaded/files/2017/06/22/a8fdf120-5eb1-4a84-af4a-1236a08a765b__asset-contains.json > Asset IN > > https://reviews.apache.org/media/uploaded/files/2017/06/22/e2096d10-ea4d-4306-b68d-4f5c531fab3c__asset-in.json > Asset like > > https://reviews.apache.org/media/uploaded/files/2017/06/22/6774768f-26ba-4386-b0b6-8ea682813dc7__asset-like.json > Hive table (date comparison) > > https://reviews.apache.org/media/uploaded/files/2017/06/22/d6e54af0-f27f-4c6f-b9d3-4e3a0eb258ac__hive_table-date_2.json > Hive table date > > https://reviews.apache.org/media/uploaded/files/2017/06/22/f3558280-60bd-49dd-92c6-c1176f86bc70__hive_table-date.json > Hive Table Like > > https://reviews.apache.org/media/uploaded/files/2017/06/22/f07517dd-1c6c-4d2d-8bfd-3d494506b23c__hive_table-like.json > Type and tag > > https://reviews.apache.org/media/uploaded/files/2017/06/22/83768696-aa5c-4c08-a776-143753eb4548__type-tag.json > > > Thanks, > > Apoorv Naik > >
