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




repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java
Line 78 (original), 84 (patched)
<https://reviews.apache.org/r/73128/#comment311568>

    Can field isNestedQuery be replaced with following method?
      public boolean isNestedQuery() {
        return parent != null;
      }



repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java
Line 123 (original), 147 (patched)
<https://reviews.apache.org/r/73128/#comment311569>

    nit: itemAssignmentExprs will not be null, ref #50.



repository/src/main/java/org/apache/atlas/query/executors/GremlinClauseToTraversalTranslator.java
Lines 88 (patched)
<https://reviews.apache.org/r/73128/#comment311570>

    subTraversals could be an empty list - in iterations after #90, #99. 
Consider one of the following to address this:
     - move #58 inside for-loop at #60
     - update 'if' at #88 and #97 to check for empty list
       if (subTraversals != null && !subTraversals.isEmpty())
     - set subTraversals to null in #90 and #99


- Madhan Neethiraj


On Jan. 14, 2021, 6:46 a.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73128/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2021, 6:46 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Nikhil Bonte, Nixon Rodrigues, Pinal 
> Shah, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-2932
>     https://issues.apache.org/jira/browse/ATLAS-2932
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Approach**
> General Approach:
> - Ensure that Gremlin-based implementation continues to function. This will 
> ensure backward compatibility and will guarad against regressions or behavior 
> deviations, if any, are introduced with the new implementation.
> - Continue using existing _GremlinQueryComposer_. 
> - Translate the clauses to _AtlasGraphTraversal_.
> - Projects that were earlier handled in Gremlin query are now handled in a 
> separate Java class.
> 
> Details:
> - Modified: _GremlinQueryComposer_: Continue composing queries using existing 
> claues. Additions alone.
> - New: _DSLQueryExecutor_ Instantiates old or new executor based on flag.
> - Modifed: _EntityDiscoveryService_ uses the new exectors.
> - New _GremlinToTraversalTranslator_: Converts Gremlin clauses to 
> _AtlasGraphTraversal_.
> - Modified: _AtlasGraphTraversal_: Additional methods.
> - New: _SelectClauseProjections_: All queries with select operations.
> 
> **Configuration**
> _atlas.dsl.executor.traversal_ 
> - true: Uses the traversal-based implementation for query execution.
> - false: Uses the script-engine based implementation for query execution.
> 
> **Credits**
> - Apoorv Naik (apoorvnaik): Original implementation.
> - Nikhil Bonte (nbonte): Continued on Apoorv's implementation and addressed 
> short-comings.
> 
> 
> Diffs
> -----
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java 
> c016f6340 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphTraversal.java
>  881bb1e0f 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java
>  0dd573b89 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphTraversal.java
>  c33c4f4d0 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java ea9f26d47 
>   
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
>  01a6c303a 
>   repository/src/main/java/org/apache/atlas/query/AtlasDSL.java b8a744b35 
>   repository/src/main/java/org/apache/atlas/query/DSLVisitor.java 700da955f 
>   repository/src/main/java/org/apache/atlas/query/GremlinClause.java 
> 9704b0f9e 
>   repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java 
> 9f30e4dc0 
>   repository/src/main/java/org/apache/atlas/query/GremlinQuery.java 531f7ae86 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 
> 36b514e84 
>   repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java 
> 969fcd2b1 
>   
> repository/src/main/java/org/apache/atlas/query/executors/DSLQueryExecutor.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/query/executors/GremlinClauseToTraversalTranslator.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/query/executors/ScriptEngineBasedExecutor.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/query/executors/SelectClauseProjections.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/query/executors/TraversalBasedExecutor.java
>  PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java 
> 3bb3b07bf 
> 
> 
> Diff: https://reviews.apache.org/r/73128/diff/9/
> 
> 
> Testing
> -------
> 
> **Unit Tests**
> - Additional tests.
> - _GremlinCompoerTest_ continues to validate the clause creation.
> - Improved validation for _minMaxCount_ tests.
> - Refactored: _TableValidator_ Validates output from a query with select 
> claues.
> 
> **Volume Tests**
> - Old implementation: 528 queries take over 10 mins to execute.
> - New implemeentations: 528 queries execute in 7 secs.
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>

Reply via email to