You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Madhan Neethiraj <ma...@apache.org> on 2018/01/04 03:24:41 UTC
Review Request 64943: ATLAS-2332: support for attributes having nested
collection datatype
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64943/
-----------------------------------------------------------
Review request for atlas.
Bugs: ATLAS-2332
https://issues.apache.org/jira/browse/ATLAS-2332
Repository: atlas
Description
-------
Updated type-def and instance modules to enable attributes with nested collection datatypes, like map<string,array<string>>, array<map<string,string>>
Diffs
-----
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java e507a8ad
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/StringListSerializer.java fa6f5fda
intg/src/test/java/org/apache/atlas/TestUtilsV2.java bbccf77c
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3e602431
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java f6a15b69
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 706e7373
repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java e1047220
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java 1f8b9fdb
Diff: https://reviews.apache.org/r/64943/diff/1/
Testing
-------
added unit tests to validate handling of attributes with nested collection datatypes
Thanks,
Madhan Neethiraj
Re: Review Request 64943: ATLAS-2332: support for attributes having
nested collection datatype
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64943/#review194790
-----------------------------------------------------------
Ship it!
Ship It!
- Sarath Subramanian
On Jan. 3, 2018, 7:24 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64943/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2018, 7:24 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2332
> https://issues.apache.org/jira/browse/ATLAS-2332
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated type-def and instance modules to enable attributes with nested collection datatypes, like map<string,array<string>>, array<map<string,string>>
>
>
> Diffs
> -----
>
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java e507a8ad
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/StringListSerializer.java fa6f5fda
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java bbccf77c
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3e602431
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java f6a15b69
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 706e7373
> repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java e1047220
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java 1f8b9fdb
>
>
> Diff: https://reviews.apache.org/r/64943/diff/1/
>
>
> Testing
> -------
>
> added unit tests to validate handling of attributes with nested collection datatypes
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 64943: ATLAS-2332: support for attributes having
nested collection datatype
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Jan. 5, 2018, 11:03 a.m., Graham Wallis wrote:
> > Regarding serialization with JanusGraph:
> >
> > I think JanusGraph's StandardSerializer has support for Lists - in the xxxArraySerializer classes (registered as 22 thru 30). But I don't see anything in StandardSerializer that handles Maps....which seems to contradict your email so is slightly confusing. Presumably we (Atlas) work our way through the Map delegating serialization of each entry to the appopriate serializer?
> >
> > I don't think there is anything (as standard) in JanusGraph's StandardSerializer that would help with BigDecimal, BigInteger or TypeCategorySerializer - but for any Atlas specific classes we could register Custom Serializers with JanusGraph - i.e. located above offset 100. TypeCategory is only a skinny layer over EnumSerializer, so not sure it is worth the effort? I think we could just keep our existing Atlas BigDecimalSerializer, BigIntegerSerializer and TypeCategorySerializer.
> >
> >
> >
> > In terms of Atlas classes/methods:
> >
> > Why do we have both GraphHelper:getMapValueProperty() and EntityGraphMapper:getMapValueProperty()? Should one delegate to the other?
> > Also, for anything that is not a Reference, one of the above methods explicitly handles List and Map property values and returns List.class and Map.class objects, whereas the other returns an Object.class. Should this be consistent across both methods?
Graham - Titan-0.5.4 has built-in support for serialization of List objects (see StandardAttributeHandling.java - https://github.com/thinkaurelius/titan/blob/titan05/titan-core/src/main/java/com/thinkaurelius/titan/graphdb/database/serialize/StandardAttributeHandling.java#L36); and this was removed in JanusGraph (and Titan-1.0). Changes in this patch are inline with JanusGraph support for HashMap objects (StandardSerializer.java - https://github.com/JanusGraph/janusgraph/blob/ab53b5d392a39e82553063ad7c62b10d51e55ec8/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/serialize/StandardSerializer.java#L132).
Perhaps we can replace BigIntegerSerializer, BigDecimalSerializer and TypeCategorySerializer with SerializableSerializer?
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64943/#review194849
-----------------------------------------------------------
On Jan. 4, 2018, 3:24 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64943/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 3:24 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2332
> https://issues.apache.org/jira/browse/ATLAS-2332
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated type-def and instance modules to enable attributes with nested collection datatypes, like map<string,array<string>>, array<map<string,string>>
>
>
> Diffs
> -----
>
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java e507a8ad
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/StringListSerializer.java fa6f5fda
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java bbccf77c
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3e602431
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java f6a15b69
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 706e7373
> repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java e1047220
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java 1f8b9fdb
>
>
> Diff: https://reviews.apache.org/r/64943/diff/1/
>
>
> Testing
> -------
>
> added unit tests to validate handling of attributes with nested collection datatypes
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 64943: ATLAS-2332: support for attributes having
nested collection datatype
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64943/#review194849
-----------------------------------------------------------
Regarding serialization with JanusGraph:
I think JanusGraph's StandardSerializer has support for Lists - in the xxxArraySerializer classes (registered as 22 thru 30). But I don't see anything in StandardSerializer that handles Maps....which seems to contradict your email so is slightly confusing. Presumably we (Atlas) work our way through the Map delegating serialization of each entry to the appopriate serializer?
I don't think there is anything (as standard) in JanusGraph's StandardSerializer that would help with BigDecimal, BigInteger or TypeCategorySerializer - but for any Atlas specific classes we could register Custom Serializers with JanusGraph - i.e. located above offset 100. TypeCategory is only a skinny layer over EnumSerializer, so not sure it is worth the effort? I think we could just keep our existing Atlas BigDecimalSerializer, BigIntegerSerializer and TypeCategorySerializer.
In terms of Atlas classes/methods:
Why do we have both GraphHelper:getMapValueProperty() and EntityGraphMapper:getMapValueProperty()? Should one delegate to the other?
Also, for anything that is not a Reference, one of the above methods explicitly handles List and Map property values and returns List.class and Map.class objects, whereas the other returns an Object.class. Should this be consistent across both methods?
- Graham Wallis
On Jan. 4, 2018, 3:24 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64943/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 3:24 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2332
> https://issues.apache.org/jira/browse/ATLAS-2332
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated type-def and instance modules to enable attributes with nested collection datatypes, like map<string,array<string>>, array<map<string,string>>
>
>
> Diffs
> -----
>
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java e507a8ad
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/StringListSerializer.java fa6f5fda
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java bbccf77c
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3e602431
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java f6a15b69
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 706e7373
> repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java e1047220
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java 1f8b9fdb
>
>
> Diff: https://reviews.apache.org/r/64943/diff/1/
>
>
> Testing
> -------
>
> added unit tests to validate handling of attributes with nested collection datatypes
>
>
> Thanks,
>
> Madhan Neethiraj
>
>