> On Feb. 23, 2017, 7:14 a.m., Madhan Neethiraj wrote:
> > Consider the following scenario:
> > - create hive_table 'employee'
> > - create hive_column 'name'; set attribute 'table' to 'employee' table
> > created above
> > - now 'employee.columns' attribute lists 'name' - good!
> > - create hive_table 'person'
> > - edit the hive_column created above ('name'), to set attribute 'table' to
> > 'person' table
> > - now 'person.columns' attribute lists 'name' - good!
> >
> > However, 'name' attribute is still seen in 'employee' table. This should
> > not be the case, right? Can you please review?
>
> David Kantor wrote:
> I think what you are seeing in this use case is the result of having soft
> deletes enabled, and based on my understanding of how soft deletion is
> implemented, it is the expected behavior. The graph edge which represents
> the reference from hive_table(employee).columns to hive_column(name) is in
> fact deleted. With soft deletes enabled, deleting an edge or vertex amounts
> to setting its __state property to DELETED. However, the edge ID for the
> deleted edge is *not* deleted from the hive_table(employee).columns property
> value - see the comments in DeleteHandlerV1.deleteEdgeBetweenVertices() lines
> 465-467 (which were copied from DeleteHandler.deleteEdgeBetweenVertices()
> lines 349-351). When soft deletes are enabled, shouldUpdateReverseAttribute
> is false, so the logic to remove the edge ID for the deleted edge from the
> reference property value is not executed, and the edge ID for the deleted
> edge remains in the property. When the entity is retrieved, there is no
> logic that I could
find which filters out soft deleted edges for a multiplicity-many reference
(i.e. an attribute that is an array of CLASS (legacy) or OBJECT_ID_TYPE (new)
elements) in either the legacy code path (GraphToTypedInstanceMapper) or in the
new code path (EntityGraphRetriever). The specific logic to not delete the
edge ID from the vertex property (and soft deletes in general) was implemented
by Shwetha Shivalingamurthy. Perhaps she could comment on this.
Thanks for the details. I think you are right - this seems to be the result of
how soft-delete is handled. We will have to think through more on how to handle
this state, to avoid potential confusion for users.
Rest of the patch looks good.
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56860/#review166483
-----------------------------------------------------------
On Feb. 22, 2017, 11:48 p.m., David Kantor wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56860/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 11:48 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1552
> https://issues.apache.org/jira/browse/ATLAS-1552
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Automatic update of inverse references in V2 (V1?) code path.
>
>
> Diffs
> -----
>
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java
> 84e17cb364fbf6d07b2211d85e336d8146fdb684
>
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java
> 49d5a080158e3e54fd917a13f743f1cf495e66b0
>
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
> 09f69dbe6f519d84ae8e223532fc5adaa411ed9b
>
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasDeleteHandlerV1Test.java
> 555f0acc22c4bec67f1a2a4064acad66e939747a
>
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/InverseReferenceUpdateHardDeleteV1Test.java
> PRE-CREATION
>
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/InverseReferenceUpdateSoftDeleteV1Test.java
> PRE-CREATION
>
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/InverseReferenceUpdateV1Test.java
> PRE-CREATION
>
> Diff: https://reviews.apache.org/r/56860/diff/
>
>
> Testing
> -------
>
> Ran all unit and integration tests with no regressions.
>
>
> Thanks,
>
> David Kantor
>
>