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




repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java
Lines 284 (patched)
<https://reviews.apache.org/r/73128/#comment311579>

    If 'attributeType.getNormalizedValue(attrVal)' is null, #283 will cause 
NPE. i.e. when attrValue is not of expected format.
    
    If attrValue is not validated earlier, I suggest to handle NULL condition 
here.



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

    Can you review how this handles nested and/or clauses? Like:
      hive_table where (name = 'test_tbl' or ((owner = 'hive' and name = 
'my_tbl) or (owner = 'sys' or createTime > "2021-01-01'T'00:00:00.000'Z'")))


- Madhan Neethiraj


On Jan. 16, 2021, 7:06 a.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73128/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2021, 7:06 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, 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/AtlasGraphTraversal.java
>  881bb1e0f 
>   
> 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 ca0b4be91 
>   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 
> 87c8bd2ee 
>   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/BaseDSLComposer.java 
> PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java 
> 958c4281c 
>   
> repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 
> 6c69855b7 
>   repository/src/test/java/org/apache/atlas/query/TraversalComposerTest.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/73128/diff/11/
> 
> 
> 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