> On Sept. 14, 2022, 11:11 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > > Lines 478 (patched) > > <https://reviews.apache.org/r/74109/diff/4/?file=2269974#file2269974line478> > > > > Use of HashSet here will eliminate duplicate rows. Is this by choice? > > Wouldn't this result in incorrect handling of pagination (offset)?
These elimination happens after handling pagination and getting the resultSet. but yes, you are correct, from user end it will be confusing, that they have requested for 10, and have got less. > On Sept. 14, 2022, 11:11 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > > Lines 507 (patched) > > <https://reviews.apache.org/r/74109/diff/4/?file=2269974#file2269974line507> > > > > return here would result in skipping of scrubSearchResults() - which is > > incorrect. Please review and update. > > > > Instead of introducing #471 - #509, consider updating existing for loop > > below at #549, like: > > > > for (AtlasVertex atlasVertex : resultList) { > > if (searchContext.excludeHeaderAttributes()) { > > // code from #475 - #506 > > } else { > > // existing code > > ... > > } > > } Correct, but we are skipping many attributes, i.e classifications and also we don't have 'entities' (we just have the 'attributes') in AtlasSearchResult I thought to align this with DSL search, Select Clause where AtlasSearchResult has only 'attributes' field. If we add support to scrubSearchResult - we need to create 'entities' from what information - we need to get 'guid' from the vertex - we need to get 'classifications' - we need to add 'referredEntities' getting classifications and referredEntities will reduce the performance which we are expecting with this improvement. I will try implementing this once. Any suugestions? > On Sept. 14, 2022, 11:11 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java > > Line 1356 (original), 1356 (patched) > > <https://reviews.apache.org/r/74109/diff/4/?file=2269976#file2269976line1356> > > > > Consider retaining getVertexAttribute() as private, based on comment in > > EntityDiscoveryService.java #494. comment on #494, By entityRetriever.getVertexAttribute(); do you mean entityRetriever.getVertexAttribute(vertex, attribute); ? We dont have any existing method, other than this which returns the value of attribute. And to access this in EntityDiscoveryService, we need to make this as public - Pinal ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74109/#review224671 ----------------------------------------------------------- On Sept. 14, 2022, 12:11 p.m., Pinal Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74109/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2022, 12:11 p.m.) > > > Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra. > > > Bugs: ATLAS-4671 > https://issues.apache.org/jira/browse/ATLAS-4671 > > > Repository: atlas > > > Description > ------- > > **Issue** : Basic search has AtlasEntityHeader of each entity in the response. > AtlasEntityHeader has many attributes including classification and terms. > hence for each attribute, it will request janusgraph > > **Approach** : To overcome, we can add a flag to exclude other attributes and > add only selected attributes from 'attributes' field in the response > > **Request**: { > "excludeDeletedEntities": true, > "includeSubClassifications": true, > "includeSubTypes": true, > "includeClassificationAttributes": true, > "limit": 25, > "offset": 0, > "typeName": "hdfs_path", > "attributes": ["path", "name"], > "excludeHeaderAttributes": "true" > } > > **Response**: { > "queryType": "BASIC", > "searchParameters": { > "typeName": "hdfs_path", > "excludeDeletedEntities": true, > "includeClassificationAttributes": true, > "includeSubTypes": true, > "includeSubClassifications": true, > "limit": 25, > "offset": 0, > "attributes": ["path", "name"] > }, > "attributes": { > "name": ["path", "name"], > "values": [ > ["/data/warehouse/customer", "customer"], > ["/data/warehouse/sales", "sales"] > ] > }, > "approximateCount": 2 > } > > > Diffs > ----- > > > intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java > 79f5aae0d > intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java > 78fb4a48f > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > 8fbc22fa0 > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java > 01954d07e > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java > a8fe5a762 > > repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java > fbc739652 > > > Diff: https://reviews.apache.org/r/74109/diff/5/ > > > Testing > ------- > > Unit testcases added > Precommit : > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/ > > Performance Readings: > Basic Search on 20K entities, limit 1000 > > > Thanks, > > Pinal Shah > >
