atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apoorv Naik <naik.apo...@gmail.com>
Subject Re: Review Request 65001: Atlas Advanced Search: Phase 3: Count, Sum, Max, Min with Group By Implementation
Date Sat, 06 Jan 2018 01:41:47 GMT

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




repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java
Lines 856 (patched)
<https://reviews.apache.org/r/65001/#comment274003>

    Update variable names for more clarity



repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java
Lines 861 (patched)
<https://reviews.apache.org/r/65001/#comment274004>

    Same here. Rename assign2 -> assign



repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java
Lines 93 (patched)
<https://reviews.apache.org/r/65001/#comment274005>

    equals -> equalsIgnoreCase



repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParser.java
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/65001/#comment274006>

    Remove user information from the file header



repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParserBaseVisitor.java
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/65001/#comment274007>

    Same here



repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParserVisitor.java
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/65001/#comment274008>

    Same here



repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java
Lines 282 (patched)
<https://reviews.apache.org/r/65001/#comment274009>

    I think we should disallow such constructs, 
    
    The query is ambiguous when used with = operator,
    
    I'm not sure whether the user wants to do an exact match for the array of values or wants
to match either value in that array.
    
    We should update the grammar to have a IN clause for such cases


- Apoorv Naik


On Jan. 6, 2018, 1:26 a.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65001/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2018, 1:26 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-2229
>     https://issues.apache.org/jira/browse/ATLAS-2229
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> This is the subsequent commit for the new Antlr-based DSL implementation.
> 
> This builds on top of what is already present.
> 
> **Approach**
> The implementation of these operators does not impact the gremlin queries generated without
these operators. Alternatively stated, the approach applies the new implementation only when
on or any of these clauses are present within the DSL query.
> 
> Basic Gremlin is constructed. The 'select' clause is processed for one of these operations.
Additional closures are constructed and the filtered part of the query is passed to the closure.
> 
> **Bug fixes**
> - Updates include pagination across results. This causes the query to fire again with
the appropriate values for _limit_ and _offset_ clauses.
> - Fixed handling of date and boolean (true and false) values in _where_ clause.
> - Error handling during parsing.
> - Handling errors for invalid queries.
> 
> **Documentation**
> - Updated documentation, currently in draft. Separate patch will include updates to TWIKI.
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java cbe9a5b5

>   repository/src/main/java/org/apache/atlas/query/AtlasDSL.java e4c81653 
>   repository/src/main/java/org/apache/atlas/query/DSLVisitor.java a21e8843 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 6af1db67

>   repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java fe1c5bdc 
>   repository/src/main/java/org/apache/atlas/query/QueryParams.java 5af8bc7b 
>   repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLLexer.java 77c66c99

>   repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParser.g4 72d08e19 
>   repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParser.java e557c4af

>   repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParserBaseVisitor.java
4423c0e9 
>   repository/src/main/java/org/apache/atlas/query/antlr4/AtlasDSLParserVisitor.java 8c59d7a6

>   repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java af4dc3e0 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java ccf1cb29

>   repository/src/test/java/org/apache/atlas/services/EntityDiscoveryServiceTest.java
8a06d949 
> 
> 
> Diff: https://reviews.apache.org/r/65001/diff/1/
> 
> 
> Testing
> -------
> 
> **Unit test**
> - Updated _GremlinComposerTest_ to include the new operations.
> - Added 25+ new tests to _DSLQueriesTest_. See section _minMaxCountProvider_.
> 
> **Functional testing**
> - Via web.
> - Verified using hdfs_path samples.
> - Verified using quick_start dataset.
> - Verified using typical hive_db dataset (with 110 tables, each with 87 columns).
> 
> **REST API testing**
> - V1 and V2 end points, via CURL.
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message