----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72713/#review221417 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java Line 526 (original), 534 (patched) <https://reviews.apache.org/r/72713/#comment310382> Block #534 - #557 is executed only for TIME_RANGE operator. This doesn't look right, given this block was executed for all operators before thisi patch. Please review and update. repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java Lines 797 (patched) <https://reviews.apache.org/r/72713/#comment310383> Consider moving #797, #798, #807 - #815 under a "case AtlasBaseTypeDef.ATLAS_TYPE_DATE:", like: case AtlasBaseTypeDef.ATLAS_TYPE_DATE: if (op == SearchParameters.Operator.TIME_RANGE && StringUtils.isNotEmpty(attrVal)) { String[] parts = attrVal.split(ATTRIBUTE_VALUE_DELIMITER); String rangeStart = parts.length > 0 ? parts[0]; String rangeEnd = parts.length > 1 ? parts[1]; attrValue = StringUtils.isEmpty(rangeStart) ? null : Long.parseLong(rangeStart); attrValue2 = StringUtils.isEmpty(rangeEnd) ? null : Long.parseLong(rangeEnd); } else { attrValue = StringUtils.isEmpty(attrVal) ? null : Long.parseLong(attrVal); } repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 180 (patched) <https://reviews.apache.org/r/72713/#comment310388> getBETPredicate() => getInRangePredicate() repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 188 (patched) <https://reviews.apache.org/r/72713/#comment310390> 'final' for argument is not useful, and is a distraction in reading the code. Please remove. repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 189 (patched) <https://reviews.apache.org/r/72713/#comment310389> Instead of returning null, I suggest: return generatePredicate(attrName, attrVal, attrVal, attrClass); repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 195 (patched) <https://reviews.apache.org/r/72713/#comment310391> add "attrVal2 == null" as well. repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Line 767 (original), 800 (patched) <https://reviews.apache.org/r/72713/#comment310392> Interfaces can have 'default' methods, so there is no need to change VertexAttributePredicateGenerator to an abstract class. repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 803 (patched) <https://reviews.apache.org/r/72713/#comment310387> Consider renaming the new method as generateInRangePredicate() repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 1008 (patched) <https://reviews.apache.org/r/72713/#comment310385> Mark 'value2' as final, and initialize to null in constructors at #1010 and #1016. repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 1021 (patched) <https://reviews.apache.org/r/72713/#comment310384> value => rangeStart value2 => rangeEnd repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java Lines 1075 (patched) <https://reviews.apache.org/r/72713/#comment310386> getBETPredicate() => getInRangePredicate() - Madhan Neethiraj On July 30, 2020, 5:57 a.m., chaitali wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72713/ > ----------------------------------------------------------- > > (Updated July 30, 2020, 5:57 a.m.) > > > Review request for atlas, Jayendra Parab, Madhan Neethiraj, Nixon Rodrigues, > and Sarath Subramanian. > > > Bugs: ATLAS-3892 > https://issues.apache.org/jira/browse/ATLAS-3892 > > > Repository: atlas > > > Description > ------- > > 1.This patch implements the search history feature where from UI we will > receive below filters. > 2.In backend we have introduced a new operator "BETWEEN" where all the below > 7 operators will be converted to BETWEEN operator. > 3.There will be a slight change in the JSON where previously we were passing > the Attribute value in Search parameters from the UI but for below filters > we will not pass any attributeValue except for "custom range" filter we > provide attribute value > for eg ;"attributeValue":"1593541800000,1595442600000" . > > New 7 filters : > > 1.last 7 days. > 2.Last month. > 3.Last 30 days. > 4.Today. > 5.Yesterday. > 6.Custom Range. > 7.This Month. > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/repository/Constants.java a71787bc0 > intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java > fcc4494f2 > repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java > c9a605355 > repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java > 5069d78c8 > > repository/src/test/java/org/apache/atlas/discovery/EntitySearchProcessorTest.java > 8e42d1700 > > > Diff: https://reviews.apache.org/r/72713/diff/4/ > > > Testing > ------- > > Tested with below json: > eg : > { > "excludeDeletedEntities": true, > "includeSubClassifications": true, > "includeSubTypes": true, > "includeClassificationAttributes": true, > "entityFilters": { > "condition": "AND", > "criterion": [ > { > "attributeName": "__timestamp", > "operator": "THISMONTH", > "attributeValue": "" > } > ] > }, > "tagFilters": null, > "attributes": [ > "__timestamp" > ], > "limit": 25, > "offset": 0, > "typeName": "Table", > "classification": null, > "termName": null > } > eg: Only For custom range filter we provide attribute value. > { > "excludeDeletedEntities": true, > "includeSubClassifications": true, > "includeSubTypes": true, > "includeClassificationAttributes": true, > "entityFilters": { > "condition": "AND", > "criterion": [ > { > "attributeName": "createTime", > "operator": "CUSTOM_RANGE", > "attributeValue": "1593541800000,1595442600000" > } > ] > }, > "tagFilters": null, > "attributes": [ > "createTime" > ], > "limit": 25, > "offset": 0, > "typeName": "Table", > "classification": null, > "termName": null > } > > * Added Test cases to check new processDateRange method functionality > > > Thanks, > > chaitali > >
