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 <am...@hortonworks.com> on 2018/01/23 21:34:35 UTC

Review Request 65297: Advanced Search DSL: Support System Attributes within Query

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

Review request for atlas and Apoorv Naik.


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


Repository: atlas


Description
-------

**Background**
Current implementation of DSL query does not support system attributes within the query.

**Analysis**
The _AtlasEntity_ does not recognize system attributes as attributes. There isn't a way to fetch all the system attributes for a given entity.

During vertex creation, system attributes are added to the vertex. They are part of the vertex, hence the only way to fetch them is by having a vertex.

Adding vertex to _RegistryBasedLook_ will create additional dependency. It will also force instantiation of an entity just for doing existence checks.

**Approach & Impact**
_query.RegistryBasedLookup_:
- Modify _hasAttribute_ to recognize system attributes as part of any type. Treat system attributes as primitive types, modify _isPrimitive_.


Diffs
-----

  repository/src/main/java/org/apache/atlas/query/DSLVisitor.java 9a213a32 
  repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 95ba4617 
  repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java eef3463e 
  repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java c44eea3b 
  repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 5b394cb2 


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


Testing
-------

**Unit tests**
- Updated _GremlinQueryComposerTest_ to include additional tests.
- Updated _DSLQueriesTest_ to include additional tests.

**Funtional tests**
- Executed queries from within web ui.


Thanks,

Ashutosh Mestry


Re: Review Request 65297: Advanced Search DSL: Support System Attributes within Query

Posted by Ashutosh Mestry <am...@hortonworks.com>.

> On Jan. 24, 2018, 5:17 a.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/query/DSLVisitor.java
> > Line 44 (original), 41 (patched)
> > <https://reviews.apache.org/r/65297/diff/3/?file=1944681#file1944681line44>
> >
> >     Now that visitIsClause & visitHasClause are no more overridden, the base implementation in AtlasDSLParserBaseVisitor will be executed by the parser. Please review if this is okay.

Yes. This was done after some discussion. Our tests pass after this change (without any other change).


> On Jan. 24, 2018, 5:17 a.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java
> > Lines 61 (patched)
> > <https://reviews.apache.org/r/65297/diff/3/?file=1944683#file1944683line61>
> >
> >     It will be efficient to use a set for this lookup:
> >     
> >     private static final Set<String> SYSTEM_ATTRIBUTES = new HashSet<>(Arrays.asList(Constants.GUID_PROPERTY_KEY,
> >                                                                                      Constants.MODIFIED_BY_KEY,
> >                                                                                      Constants.CREATED_BY_KEY,
> >                                                                                      Constants.STATE_PROPERTY_KEY,
> >                                                                                      Constants.TIMESTAMP_PROPERTY_KEY,
> >                                                                                      Constants.MODIFICATION_TIMESTAMP_PROPERTY_KEY));
> >     
> >     private boolean isSystemAttribute(String s) {
> >       return SYSTEM_ATTRIBUTES.contains(s);
> >     }

Good point.


- Ashutosh


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


On Jan. 23, 2018, 9:34 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65297/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2018, 9:34 p.m.)
> 
> 
> Review request for atlas and Apoorv Naik.
> 
> 
> Bugs: ATLAS-2418
>     https://issues.apache.org/jira/browse/ATLAS-2418
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> Current implementation of DSL query does not support system attributes within the query.
> 
> **Analysis**
> The _AtlasEntity_ does not recognize system attributes as attributes. There isn't a way to fetch all the system attributes for a given entity.
> 
> During vertex creation, system attributes are added to the vertex. They are part of the vertex, hence the only way to fetch them is by having a vertex.
> 
> Adding vertex to _RegistryBasedLook_ will create additional dependency. It will also force instantiation of an entity just for doing existence checks.
> 
> **Approach & Impact**
> _query.RegistryBasedLookup_:
> - Modify _hasAttribute_ to recognize system attributes as part of any type. Treat system attributes as primitive types, modify _isPrimitive_.
> - These are the attributes treated as system attributes:
>   - __guid
>   - __typeName
>   - __createTime
>   - __updateTime
>   - __createdBy
>   - __updatedBy
> 
> **New Queries Supported**
> ```
> hive_db has __state
> hive_db where hive_db has __state
> hive_db as d where d.__state = 'ACTIVE'
> hive_db select __guid
> hive_db where __state = 'ACTIVE' select name, __guid, __state
> hive_table where __state = 'DELETED'
> ```
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/query/DSLVisitor.java 9a213a32 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 95ba4617 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java eef3463e 
>   repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java c44eea3b 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 5b394cb2 
> 
> 
> Diff: https://reviews.apache.org/r/65297/diff/3/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> - Updated _GremlinQueryComposerTest_ to include additional tests.
> - Updated _DSLQueriesTest_ to include additional tests.
> 
> **Funtional tests**
> - Executed queries from within web ui.
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 65297: Advanced Search DSL: Support System Attributes within Query

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




repository/src/main/java/org/apache/atlas/query/DSLVisitor.java
Line 44 (original), 41 (patched)
<https://reviews.apache.org/r/65297/#comment275612>

    Now that visitIsClause & visitHasClause are no more overridden, the base implementation in AtlasDSLParserBaseVisitor will be executed by the parser. Please review if this is okay.



repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java
Lines 61 (patched)
<https://reviews.apache.org/r/65297/#comment275613>

    It will be efficient to use a set for this lookup:
    
    private static final Set<String> SYSTEM_ATTRIBUTES = new HashSet<>(Arrays.asList(Constants.GUID_PROPERTY_KEY,
                                                                                     Constants.MODIFIED_BY_KEY,
                                                                                     Constants.CREATED_BY_KEY,
                                                                                     Constants.STATE_PROPERTY_KEY,
                                                                                     Constants.TIMESTAMP_PROPERTY_KEY,
                                                                                     Constants.MODIFICATION_TIMESTAMP_PROPERTY_KEY));
    
    private boolean isSystemAttribute(String s) {
      return SYSTEM_ATTRIBUTES.contains(s);
    }


- Madhan Neethiraj


On Jan. 23, 2018, 9:34 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65297/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2018, 9:34 p.m.)
> 
> 
> Review request for atlas and Apoorv Naik.
> 
> 
> Bugs: ATLAS-2418
>     https://issues.apache.org/jira/browse/ATLAS-2418
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Background**
> Current implementation of DSL query does not support system attributes within the query.
> 
> **Analysis**
> The _AtlasEntity_ does not recognize system attributes as attributes. There isn't a way to fetch all the system attributes for a given entity.
> 
> During vertex creation, system attributes are added to the vertex. They are part of the vertex, hence the only way to fetch them is by having a vertex.
> 
> Adding vertex to _RegistryBasedLook_ will create additional dependency. It will also force instantiation of an entity just for doing existence checks.
> 
> **Approach & Impact**
> _query.RegistryBasedLookup_:
> - Modify _hasAttribute_ to recognize system attributes as part of any type. Treat system attributes as primitive types, modify _isPrimitive_.
> - These are the attributes treated as system attributes:
>   - __guid
>   - __typeName
>   - __createTime
>   - __updateTime
>   - __createdBy
>   - __updatedBy
> 
> **New Queries Supported**
> ```
> hive_db has __state
> hive_db where hive_db has __state
> hive_db as d where d.__state = 'ACTIVE'
> hive_db select __guid
> hive_db where __state = 'ACTIVE' select name, __guid, __state
> hive_table where __state = 'DELETED'
> ```
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/query/DSLVisitor.java 9a213a32 
>   repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 95ba4617 
>   repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java eef3463e 
>   repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java c44eea3b 
>   repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 5b394cb2 
> 
> 
> Diff: https://reviews.apache.org/r/65297/diff/3/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> - Updated _GremlinQueryComposerTest_ to include additional tests.
> - Updated _DSLQueriesTest_ to include additional tests.
> 
> **Funtional tests**
> - Executed queries from within web ui.
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Re: Review Request 65297: Advanced Search DSL: Support System Attributes within Query

Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65297/
-----------------------------------------------------------

(Updated Jan. 25, 2018, 5:41 a.m.)


Review request for atlas and Apoorv Naik.


Changes
-------

Updates include:
- Addressed review comments.
- Minor refactoring to unit tests.


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


Repository: atlas


Description
-------

**Background**
Current implementation of DSL query does not support system attributes within the query.

**Analysis**
The _AtlasEntity_ does not recognize system attributes as attributes. There isn't a way to fetch all the system attributes for a given entity.

During vertex creation, system attributes are added to the vertex. They are part of the vertex, hence the only way to fetch them is by having a vertex.

Adding vertex to _RegistryBasedLook_ will create additional dependency. It will also force instantiation of an entity just for doing existence checks.

**Approach & Impact**
_query.RegistryBasedLookup_:
- Modify _hasAttribute_ to recognize system attributes as part of any type. Treat system attributes as primitive types, modify _isPrimitive_.
- These are the attributes treated as system attributes:
  - __guid
  - __typeName
  - __createTime
  - __updateTime
  - __createdBy
  - __updatedBy

**New Queries Supported**
```
hive_db has __state
hive_db where hive_db has __state
hive_db as d where d.__state = 'ACTIVE'
hive_db select __guid
hive_db where __state = 'ACTIVE' select name, __guid, __state
hive_table where __state = 'DELETED'
```


Diffs (updated)
-----

  repository/src/main/java/org/apache/atlas/query/DSLVisitor.java 9a213a32 
  repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java 95ba4617 
  repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java eef3463e 
  repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java c44eea3b 
  repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java 5b394cb2 


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

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


Testing
-------

**Unit tests**
- Updated _GremlinQueryComposerTest_ to include additional tests.
- Updated _DSLQueriesTest_ to include additional tests.

**Funtional tests**
- Executed queries from within web ui.


Thanks,

Ashutosh Mestry