You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Ashutosh Mestry via Review Board <no...@reviews.apache.org> on 2019/08/15 16:21:22 UTC

Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/
-----------------------------------------------------------

Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.


Bugs: ATLAS-3370
    https://issues.apache.org/jira/browse/ATLAS-3370


Repository: atlas


Description
-------

**Background**

New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
 
**Approach**

- Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
- Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
- Additional error handling.
- Additional refactoring.
- Additional unit tests.


Diffs
-----

  addons/models/0000-Area0/0010-base_model.json 1c807116d 
  graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
  graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
  intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
  intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
  intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
  repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
  repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
  repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
  repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
  repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
  repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
  repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
  repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 8d8e4ad92 
  repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
  repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
  repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 


Diff: https://reviews.apache.org/r/71295/diff/1/


Testing
-------

**Unit tests**
DSLQueriesTest:  246 (multiplied by 2).
GremlinQueryComposerTest: 39 tests.

**Integration tests**
Existing set.


Thanks,

Ashutosh Mestry


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/#review217223
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On Aug. 15, 2019, 4:21 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71295/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2019, 4:21 p.m.)
> 
> 
> Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3370
>     https://issues.apache.org/jira/browse/ATLAS-3370
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> 
> New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
>  
> **Approach**
> 
> - Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
> - Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
> - Additional error handling.
> - Additional refactoring.
> - Additional unit tests.
> 
> 
> Diffs
> -----
> 
>   addons/models/0000-Area0/0010-base_model.json 1c807116d 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
>   graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
>   intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
>   repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
>   repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
>   repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
>   repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
>   repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 8d8e4ad92 
>   repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
>   repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
> 
> 
> Diff: https://reviews.apache.org/r/71295/diff/1/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> DSLQueriesTest:  246 (multiplied by 2).
> GremlinQueryComposerTest: 39 tests.
> 
> **Integration tests**
> Existing set.
> 
> **Pre-commit build**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/#review217226
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On Aug. 15, 2019, 6:19 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71295/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2019, 6:19 p.m.)
> 
> 
> Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3370
>     https://issues.apache.org/jira/browse/ATLAS-3370
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> 
> New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
>  
> **Approach**
> 
> - Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
> - Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
> - Additional error handling.
> - Additional refactoring.
> - Additional unit tests.
> 
> 
> Diffs
> -----
> 
>   addons/models/0000-Area0/0010-base_model.json 1c807116d 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
>   graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
>   intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
>   repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
>   repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
>   repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
>   repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
>   repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
>   repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
>   repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
> 
> 
> Diff: https://reviews.apache.org/r/71295/diff/2/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> DSLQueriesTest:  246 (multiplied by 2).
> GremlinQueryComposerTest: 39 tests.
> 
> **Integration tests**
> Existing set.
> 
> **Pre-commit build**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/#review217243
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On Aug. 16, 2019, 6:07 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71295/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2019, 6:07 p.m.)
> 
> 
> Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3370
>     https://issues.apache.org/jira/browse/ATLAS-3370
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> 
> New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
>  
> **Approach**
> 
> - Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
> - Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
> - Additional error handling.
> - Additional refactoring.
> - Additional unit tests.
> 
> 
> Diffs
> -----
> 
>   addons/models/0000-Area0/0010-base_model.json 1c807116d 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
>   graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
>   intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
>   repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java a7ccaebc8 
>   repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
>   repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
>   repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
>   repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
>   repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
>   repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
>   repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
>   webapp/src/test/resources/json/search-parameters/combination-filters.json ea8fbdad3 
> 
> 
> Diff: https://reviews.apache.org/r/71295/diff/5/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> DSLQueriesTest:  246 (multiplied by 2).
> GremlinQueryComposerTest: 39 tests.
> 
> **Integration tests**
> Existing set.
> 
> **Pre-commit build**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Sarath Subramanian <sa...@apache.org>.

> On Aug. 16, 2019, 12:49 p.m., Sarath Subramanian wrote:
> > Ship It!

Precommit succeeded:

[INFO] Reactor Summary:
[INFO]
[INFO] Apache Atlas Server Build Tools 1.0 ................ SUCCESS [  0.579 s]
[INFO] apache-atlas 3.0.0-SNAPSHOT ........................ SUCCESS [  7.705 s]
[INFO] Apache Atlas Test Utility Tools 3.0.0-SNAPSHOT ..... SUCCESS [  9.534 s]
[INFO] Apache Atlas Integration 3.0.0-SNAPSHOT ............ SUCCESS [01:21 min]
[INFO] Apache Atlas Common 3.0.0-SNAPSHOT ................. SUCCESS [  7.653 s]
[INFO] Apache Atlas Client 3.0.0-SNAPSHOT ................. SUCCESS [  0.282 s]
[INFO] atlas-client-common 3.0.0-SNAPSHOT ................. SUCCESS [  4.150 s]
[INFO] atlas-client-v1 3.0.0-SNAPSHOT ..................... SUCCESS [  5.692 s]
[INFO] Apache Atlas Server API 3.0.0-SNAPSHOT ............. SUCCESS [  5.659 s]
[INFO] Apache Atlas Notification 3.0.0-SNAPSHOT ........... SUCCESS [ 22.093 s]
[INFO] atlas-client-v2 3.0.0-SNAPSHOT ..................... SUCCESS [  4.061 s]
[INFO] Apache Atlas Graph Database Projects 3.0.0-SNAPSHOT  SUCCESS [  0.288 s]
[INFO] Apache Atlas Graph Database API 3.0.0-SNAPSHOT ..... SUCCESS [  3.827 s]
[INFO] Graph Database Common Code 3.0.0-SNAPSHOT .......... SUCCESS [  3.351 s]
[INFO] Apache Atlas JanusGraph-HBase2 Module 3.0.0-SNAPSHOT SUCCESS [  5.270 s]
[INFO] Apache Atlas JanusGraph DB Impl 3.0.0-SNAPSHOT ..... SUCCESS [02:26 min]
[INFO] Apache Atlas Graph Database Implementation Dependencies 3.0.0-SNAPSHOT SUCCESS [  1.278 s]
[INFO] Apache Atlas Authorization 3.0.0-SNAPSHOT .......... SUCCESS [  6.818 s]
[INFO] Apache Atlas Repository 3.0.0-SNAPSHOT ............. SUCCESS [06:24 min]
[INFO] Apache Atlas UI 3.0.0-SNAPSHOT ..................... SUCCESS [ 54.262 s]
[INFO] Apache Atlas Web Application 3.0.0-SNAPSHOT ........ SUCCESS [08:02 min]
[INFO] Apache Atlas Documentation 3.0.0-SNAPSHOT .......... SUCCESS [  5.111 s]
[INFO] Apache Atlas FileSystem Model 3.0.0-SNAPSHOT ....... SUCCESS [  2.141 s]
[INFO] Apache Atlas Plugin Classloader 3.0.0-SNAPSHOT ..... SUCCESS [  6.592 s]
[INFO] Apache Atlas Hive Bridge Shim 3.0.0-SNAPSHOT ....... SUCCESS [  6.058 s]
[INFO] Apache Atlas Hive Bridge 3.0.0-SNAPSHOT ............ SUCCESS [21:44 min]
[INFO] Apache Atlas Falcon Bridge Shim 3.0.0-SNAPSHOT ..... SUCCESS [  4.558 s]
[INFO] Apache Atlas Falcon Bridge 3.0.0-SNAPSHOT .......... SUCCESS [02:51 min]
[INFO] Apache Atlas Sqoop Bridge Shim 3.0.0-SNAPSHOT ...... SUCCESS [  2.299 s]
[INFO] Apache Atlas Sqoop Bridge 3.0.0-SNAPSHOT ........... SUCCESS [02:53 min]
[INFO] Apache Atlas Storm Bridge Shim 3.0.0-SNAPSHOT ...... SUCCESS [  3.096 s]
[INFO] Apache Atlas Storm Bridge 3.0.0-SNAPSHOT ........... SUCCESS [02:43 min]
[INFO] Apache Atlas Hbase Bridge Shim 3.0.0-SNAPSHOT ...... SUCCESS [  5.475 s]
[INFO] Apache Atlas Hbase Bridge 3.0.0-SNAPSHOT ........... SUCCESS [03:05 min]
[INFO] Apache HBase - Testing Util 3.0.0-SNAPSHOT ......... SUCCESS [ 19.857 s]
[INFO] Apache Atlas Kafka Bridge 3.0.0-SNAPSHOT ........... SUCCESS [  5.818 s]
[INFO] Apache Atlas classification updater 3.0.0-SNAPSHOT . SUCCESS [  3.777 s]
[INFO] Apache Atlas Impala Hook API 3.0.0-SNAPSHOT ........ SUCCESS [  2.401 s]
[INFO] Apache Atlas Impala Bridge Shim 3.0.0-SNAPSHOT ..... SUCCESS [  2.771 s]
[INFO] Apache Atlas Impala Bridge 3.0.0-SNAPSHOT .......... SUCCESS [05:39 min]
[INFO] Apache Atlas Distribution 3.0.0-SNAPSHOT ........... SUCCESS [  1.645 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:00 h
[INFO] Finished at: 2019-08-16T12:17:09-07:00
[INFO] ------------------------------------------------------------------------


- Sarath


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/#review217245
-----------------------------------------------------------


On Aug. 16, 2019, 11:07 a.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71295/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2019, 11:07 a.m.)
> 
> 
> Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3370
>     https://issues.apache.org/jira/browse/ATLAS-3370
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> 
> New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
>  
> **Approach**
> 
> - Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
> - Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
> - Additional error handling.
> - Additional refactoring.
> - Additional unit tests.
> 
> 
> Diffs
> -----
> 
>   addons/models/0000-Area0/0010-base_model.json 1c807116d 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
>   graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
>   intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
>   repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java a7ccaebc8 
>   repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
>   repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
>   repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
>   repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
>   repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
>   repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
>   repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
>   webapp/src/test/resources/json/search-parameters/combination-filters.json ea8fbdad3 
> 
> 
> Diff: https://reviews.apache.org/r/71295/diff/5/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> DSLQueriesTest:  246 (multiplied by 2).
> GremlinQueryComposerTest: 39 tests.
> 
> **Integration tests**
> Existing set.
> 
> **Pre-commit build**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/#review217245
-----------------------------------------------------------


Ship it!




Ship It!

- Sarath Subramanian


On Aug. 16, 2019, 11:07 a.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71295/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2019, 11:07 a.m.)
> 
> 
> Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3370
>     https://issues.apache.org/jira/browse/ATLAS-3370
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> 
> New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
>  
> **Approach**
> 
> - Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
> - Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
> - Additional error handling.
> - Additional refactoring.
> - Additional unit tests.
> 
> 
> Diffs
> -----
> 
>   addons/models/0000-Area0/0010-base_model.json 1c807116d 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
>   graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
>   graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
>   intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
>   repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java a7ccaebc8 
>   repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
>   repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
>   repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
>   repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
>   repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
>   repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
>   repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
>   webapp/src/test/resources/json/search-parameters/combination-filters.json ea8fbdad3 
> 
> 
> Diff: https://reviews.apache.org/r/71295/diff/5/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> DSLQueriesTest:  246 (multiplied by 2).
> GremlinQueryComposerTest: 39 tests.
> 
> **Integration tests**
> Existing set.
> 
> **Pre-commit build**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/
-----------------------------------------------------------

(Updated Aug. 16, 2019, 6:07 p.m.)


Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.


Changes
-------

Updates include: Addressed review comments.


Bugs: ATLAS-3370
    https://issues.apache.org/jira/browse/ATLAS-3370


Repository: atlas


Description
-------

**Background**

New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
 
**Approach**

- Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
- Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
- Additional error handling.
- Additional refactoring.
- Additional unit tests.


Diffs (updated)
-----

  addons/models/0000-Area0/0010-base_model.json 1c807116d 
  graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
  graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
  intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
  intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
  intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
  repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java a7ccaebc8 
  repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
  repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
  repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
  repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
  repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
  repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
  repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
  repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
  repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
  repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
  repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
  webapp/src/test/resources/json/search-parameters/combination-filters.json ea8fbdad3 


Diff: https://reviews.apache.org/r/71295/diff/5/

Changes: https://reviews.apache.org/r/71295/diff/4-5/


Testing
-------

**Unit tests**
DSLQueriesTest:  246 (multiplied by 2).
GremlinQueryComposerTest: 39 tests.

**Integration tests**
Existing set.

**Pre-commit build**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/


Thanks,

Ashutosh Mestry


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/
-----------------------------------------------------------

(Updated Aug. 16, 2019, 6 p.m.)


Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.


Changes
-------

Updates include: Removed encoded string.


Bugs: ATLAS-3370
    https://issues.apache.org/jira/browse/ATLAS-3370


Repository: atlas


Description
-------

**Background**

New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
 
**Approach**

- Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
- Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
- Additional error handling.
- Additional refactoring.
- Additional unit tests.


Diffs (updated)
-----

  addons/models/0000-Area0/0010-base_model.json 1c807116d 
  graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
  graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
  intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
  intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
  intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
  repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java a7ccaebc8 
  repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
  repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
  repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
  repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
  repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
  repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
  repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
  repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
  repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
  repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
  repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 
  webapp/src/test/resources/json/search-parameters/combination-filters.json ea8fbdad3 


Diff: https://reviews.apache.org/r/71295/diff/4/

Changes: https://reviews.apache.org/r/71295/diff/3-4/


Testing
-------

**Unit tests**
DSLQueriesTest:  246 (multiplied by 2).
GremlinQueryComposerTest: 39 tests.

**Integration tests**
Existing set.

**Pre-commit build**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/


Thanks,

Ashutosh Mestry


Re: Review Request 71295: ATLAS-3370: Aggregation Metrics with Quick Search Do Not Result in Correct Counts

Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71295/
-----------------------------------------------------------

(Updated Aug. 15, 2019, 6:19 p.m.)


Review request for atlas, Sridhar K, Madhan Neethiraj, and Sarath Subramanian.


Changes
-------

Updates include: Re-based with latest.


Bugs: ATLAS-3370
    https://issues.apache.org/jira/browse/ATLAS-3370


Repository: atlas


Description
-------

**Background**

New patch for aggregates introduces Asset.__s_name, etc. This exposed flaw in current DSL query engine where it was using AtlasEntityType.qualifiedAttributeName instead of getVertexPropertyName. It did not matter so far since both APIs returned same values.
 
**Approach**

- Modified GremlinQueryComposer, to use output of getVertexPropertyName when adding Gremlin clauses.
- Added new method to org.query.atlas.query.Lookup to fetch vertex property name.
- Additional error handling.
- Additional refactoring.
- Additional unit tests.


Diffs (updated)
-----

  addons/models/0000-Area0/0010-base_model.json 1c807116d 
  graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java 08923f8da 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java 613a714ff 
  graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java b6889c873 
  graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java f72b41214 
  intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 8c1dbd77c 
  intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e7ec3d862 
  intg/src/main/java/org/apache/atlas/v1/model/typedef/AttributeDefinition.java d9a3a8134 
  repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 71179772a 
  repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 17c837bb3 
  repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java e74c0f5a0 
  repository/src/main/java/org/apache/atlas/query/Lookup.java ec95f5d89 
  repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java e67eeafa6 
  repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 999fe5c8b 
  repository/src/main/java/org/apache/atlas/repository/converters/TypeConverterUtil.java a248de605 
  repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java 255050d50 
  repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java bd5f525fa 
  repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java 8d7dbfbb2 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasStructDefStoreV2.java b93faf0c1 
  repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 568667054 


Diff: https://reviews.apache.org/r/71295/diff/2/

Changes: https://reviews.apache.org/r/71295/diff/1-2/


Testing
-------

**Unit tests**
DSLQueriesTest:  246 (multiplied by 2).
GremlinQueryComposerTest: 39 tests.

**Integration tests**
Existing set.

**Pre-commit build**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1357/


Thanks,

Ashutosh Mestry