sheetalshah1007 commented on code in PR #478:
URL: https://github.com/apache/atlas/pull/478#discussion_r3080824751
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1533,6 +1532,98 @@ private void mapRelationshipAttributes(AtlasEntity
entity, AtlasEntityType entit
LOG.debug("<== mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
}
+ private void mapRelationshipAttributeWithMultipleTypes(AtlasEntity entity,
AtlasEntityType entityType, String attrName, Object attrValue, AtlasVertex
vertex, EntityOperation op, EntityMutationContext context) throws
AtlasBaseException {
+ LOG.debug("==> mapRelationshipAttributeWithMultipleTypes({}, {})",
attrName, entity.getTypeName());
+ Set<String> relationshipTypeNames =
entityType.getAttributeRelationshipTypes(attrName);
+
+ if (CollectionUtils.isEmpty(relationshipTypeNames)) {
+ // Fallback to single relationship type processing
+ String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
+ mapAttribute(attribute, attrValue, vertex, op, context);
+
+ return;
+ }
+
+ if (attrValue instanceof Collection) {
+ Collection<?> relatedObjects = (Collection<?>) attrValue;
+
+ // Group related objects by their appropriate relationship type
+ // e.g., hive_table elements should use hive_table_db
relationship, iceberg_table elements should use iceberg_table_db
+ Map<String, List<Object>> elementsByRelationshipType =
groupElementsByRelationshipType(
+ relatedObjects, attrName, relationshipTypeNames);
+
+ // Process each relationship type with its filtered elements
Review Comment:
Done
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1498,29 +1498,28 @@ private void mapAttributes(AtlasStruct struct,
AtlasStructType structType, Atlas
LOG.debug("<== mapAttributes({}, {})", op, struct.getTypeName());
}
- private void mapRelationshipAttributes(AtlasEntity entity, AtlasEntityType
entityType, AtlasVertex vertex, EntityOperation op, EntityMutationContext
context) throws AtlasBaseException {
+ private void mapRelationshipAttributes(AtlasEntity entity, AtlasEntityType
entityType, AtlasVertex vertex, EntityOperation op,
+ EntityMutationContext context) throws AtlasBaseException {
LOG.debug("==> mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
if (MapUtils.isNotEmpty(entity.getRelationshipAttributes())) {
MetricRecorder metric =
RequestContext.get().startMetricRecord("mapRelationshipAttributes");
if (op.equals(CREATE)) {
for (String attrName :
entityType.getRelationshipAttributes().keySet()) {
- Object attrValue =
entity.getRelationshipAttribute(attrName);
- String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
- AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
-
- mapAttribute(attribute, attrValue, vertex, op, context);
+ Object attrValue =
entity.getRelationshipAttribute(attrName);
+ if (attrValue != null) {
Review Comment:
CREATE: We iterate every relationship-attribute name declared on the type.
Anything not set on the entity is null. If you removed the` if (attrValue !=
null) `here, you would call `mapRelationshipAttributeWithMultipleTypes` for
every missing attribute and treat “not in payload” like “clear,” which is
wrong. So the CREATE null check stays. Comment added for developer's
understanding
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1498,29 +1498,28 @@ private void mapAttributes(AtlasStruct struct,
AtlasStructType structType, Atlas
LOG.debug("<== mapAttributes({}, {})", op, struct.getTypeName());
}
- private void mapRelationshipAttributes(AtlasEntity entity, AtlasEntityType
entityType, AtlasVertex vertex, EntityOperation op, EntityMutationContext
context) throws AtlasBaseException {
+ private void mapRelationshipAttributes(AtlasEntity entity, AtlasEntityType
entityType, AtlasVertex vertex, EntityOperation op,
+ EntityMutationContext context) throws AtlasBaseException {
LOG.debug("==> mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
if (MapUtils.isNotEmpty(entity.getRelationshipAttributes())) {
MetricRecorder metric =
RequestContext.get().startMetricRecord("mapRelationshipAttributes");
if (op.equals(CREATE)) {
for (String attrName :
entityType.getRelationshipAttributes().keySet()) {
- Object attrValue =
entity.getRelationshipAttribute(attrName);
- String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
- AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
-
- mapAttribute(attribute, attrValue, vertex, op, context);
+ Object attrValue =
entity.getRelationshipAttribute(attrName);
+ if (attrValue != null) {
+ mapRelationshipAttributeWithMultipleTypes(entity,
entityType, attrName, attrValue, vertex, op, context);
+ }
}
} else if (op.equals(UPDATE) || op.equals(PARTIAL_UPDATE)) {
// relationship attributes mapping
for (String attrName :
entityType.getRelationshipAttributes().keySet()) {
if (entity.hasRelationshipAttribute(attrName)) {
- Object attrValue =
entity.getRelationshipAttribute(attrName);
- String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
- AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
-
- mapAttribute(attribute, attrValue, vertex, op,
context);
+ Object attrValue =
entity.getRelationshipAttribute(attrName);
+ if (attrValue != null) {
Review Comment:
UPDATE / PARTIAL_UPDATE: We only process names where
`entity.hasRelationshipAttribute(attrName) `is `true` — the client included
that key. The value may be null on purpose to clear that relationship
attribute. So we **do not skip** null here; we pass it through so
`mapAttribute` can apply defaults or clear, consistent with the old update loop.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1533,6 +1532,98 @@ private void mapRelationshipAttributes(AtlasEntity
entity, AtlasEntityType entit
LOG.debug("<== mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
}
+ private void mapRelationshipAttributeWithMultipleTypes(AtlasEntity entity,
AtlasEntityType entityType, String attrName, Object attrValue, AtlasVertex
vertex, EntityOperation op, EntityMutationContext context) throws
AtlasBaseException {
+ LOG.debug("==> mapRelationshipAttributeWithMultipleTypes({}, {})",
attrName, entity.getTypeName());
+ Set<String> relationshipTypeNames =
entityType.getAttributeRelationshipTypes(attrName);
+
+ if (CollectionUtils.isEmpty(relationshipTypeNames)) {
Review Comment:
**Empty relationshipTypeNames:**
`relationshipTypeNames` comes from
`getAttributeRelationshipTypes(attrName)`. When it is empty or null, it means
we cannot run the new “split by relationship type and map each group” logic —
there are no registered relationship-type keys to split on.
That does not mean there is nothing to store. The entity can still carry
relationship-attribute values in the request. We still need to write those to
the graph.
In that case we use the same behaviour as before this multi-type change:
resolve the relationship type from the value`
(AtlasEntityUtil.getRelationshipType(attrValue))`, resolve
`getRelationshipAttribute(attrName, relationType)`, and call `mapAttribute`
once. So “empty `relationshipTypeNames`” means “no multi-type split,” not “skip
mapping.” ; we still map using the legacy resolution path.
**Added branch for default behavior:**
If exactly one relationship type and `attrValue` is not a `Collection` and
not a `Map`: use that sole type from the registry and `mapAttribute` once
(default for a single ref). Comment explains map-shaped single refs skip this
and go to else.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1533,6 +1532,98 @@ private void mapRelationshipAttributes(AtlasEntity
entity, AtlasEntityType entit
LOG.debug("<== mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
}
+ private void mapRelationshipAttributeWithMultipleTypes(AtlasEntity entity,
AtlasEntityType entityType, String attrName, Object attrValue, AtlasVertex
vertex, EntityOperation op, EntityMutationContext context) throws
AtlasBaseException {
+ LOG.debug("==> mapRelationshipAttributeWithMultipleTypes({}, {})",
attrName, entity.getTypeName());
+ Set<String> relationshipTypeNames =
entityType.getAttributeRelationshipTypes(attrName);
+
+ if (CollectionUtils.isEmpty(relationshipTypeNames)) {
+ // Fallback to single relationship type processing
+ String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
+ mapAttribute(attribute, attrValue, vertex, op, context);
+
+ return;
+ }
+
+ if (attrValue instanceof Collection) {
Review Comment:
This change isn’t tied to one combination like “`hive_db + hive_table +
iceberg_table.`” It applies whenever one attribute name can point to more than
one relationship type. We sort the refs by type and map them in separate steps.
**Maps**:
The value can also be a _map_ — the field is just an _Object_. Today we only
split by type when the value is a _list_ or _set_ of refs. If it’s a map, we
don’t split it the same way; we treat it as one value and map it in one go. We
added a short note in the code. If we later need to split maps of refs the same
way as lists, we can do that as a separate change.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1533,6 +1532,98 @@ private void mapRelationshipAttributes(AtlasEntity
entity, AtlasEntityType entit
LOG.debug("<== mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
}
+ private void mapRelationshipAttributeWithMultipleTypes(AtlasEntity entity,
AtlasEntityType entityType, String attrName, Object attrValue, AtlasVertex
vertex, EntityOperation op, EntityMutationContext context) throws
AtlasBaseException {
+ LOG.debug("==> mapRelationshipAttributeWithMultipleTypes({}, {})",
attrName, entity.getTypeName());
+ Set<String> relationshipTypeNames =
entityType.getAttributeRelationshipTypes(attrName);
+
+ if (CollectionUtils.isEmpty(relationshipTypeNames)) {
+ // Fallback to single relationship type processing
+ String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
+ mapAttribute(attribute, attrValue, vertex, op, context);
+
+ return;
+ }
+
+ if (attrValue instanceof Collection) {
+ Collection<?> relatedObjects = (Collection<?>) attrValue;
+
+ // Group related objects by their appropriate relationship type
+ // e.g., hive_table elements should use hive_table_db
relationship, iceberg_table elements should use iceberg_table_db
+ Map<String, List<Object>> elementsByRelationshipType =
groupElementsByRelationshipType(
+ relatedObjects, attrName, relationshipTypeNames);
+
+ // Process each relationship type with its filtered elements
+ for (Map.Entry<String, List<Object>> entry :
elementsByRelationshipType.entrySet()) {
+ String relationshipTypeName = entry.getKey();
+ List<Object> filteredElements = entry.getValue();
+
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationshipTypeName);
+
+ if (attribute != null &&
CollectionUtils.isNotEmpty(filteredElements)) {
+ // Use the same collection type as the original (List or
Set)
+ Object filteredValue =
createCollectionOfSameType(attrValue, filteredElements);
Review Comment:
_What createCollectionOfSameType does_:
After splitting by relationship type, `filteredElements` is always built as
an `ArrayList`. The client’s original `attrValue` might still be a `List` or a
`Set`. This helper makes the value passed into `mapAttribute` match that
original kind.
So this line matters because multi-type splitting should not change the
collection type of the relationship attribute value—only which refs belong to
each relationship type when we call `mapAttribute` for that type.
Added `EntityGraphMapperMultipleRelationshipTypesTest`: contains two flows:
same mixed tables refs as `ArrayList`, then as `HashSet`; verifyRelationships
each time.
##########
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java:
##########
@@ -1533,6 +1532,98 @@ private void mapRelationshipAttributes(AtlasEntity
entity, AtlasEntityType entit
LOG.debug("<== mapRelationshipAttributes({}, {})", op,
entity.getTypeName());
}
+ private void mapRelationshipAttributeWithMultipleTypes(AtlasEntity entity,
AtlasEntityType entityType, String attrName, Object attrValue, AtlasVertex
vertex, EntityOperation op, EntityMutationContext context) throws
AtlasBaseException {
+ LOG.debug("==> mapRelationshipAttributeWithMultipleTypes({}, {})",
attrName, entity.getTypeName());
+ Set<String> relationshipTypeNames =
entityType.getAttributeRelationshipTypes(attrName);
+
+ if (CollectionUtils.isEmpty(relationshipTypeNames)) {
+ // Fallback to single relationship type processing
+ String relationType =
AtlasEntityUtil.getRelationshipType(attrValue);
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationType);
+ mapAttribute(attribute, attrValue, vertex, op, context);
+
+ return;
+ }
+
+ if (attrValue instanceof Collection) {
+ Collection<?> relatedObjects = (Collection<?>) attrValue;
+
+ // Group related objects by their appropriate relationship type
+ // e.g., hive_table elements should use hive_table_db
relationship, iceberg_table elements should use iceberg_table_db
+ Map<String, List<Object>> elementsByRelationshipType =
groupElementsByRelationshipType(
+ relatedObjects, attrName, relationshipTypeNames);
+
+ // Process each relationship type with its filtered elements
+ for (Map.Entry<String, List<Object>> entry :
elementsByRelationshipType.entrySet()) {
+ String relationshipTypeName = entry.getKey();
+ List<Object> filteredElements = entry.getValue();
+
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, relationshipTypeName);
+
+ if (attribute != null &&
CollectionUtils.isNotEmpty(filteredElements)) {
+ // Use the same collection type as the original (List or
Set)
+ Object filteredValue =
createCollectionOfSameType(attrValue, filteredElements);
+
+ LOG.debug("Processing relationship type {} for attribute
{} with {} elements",
+ relationshipTypeName, attrName,
filteredElements.size());
+
+ mapAttribute(attribute, filteredValue, vertex, op,
context);
+ }
+ }
+ } else {
+ // Single element - prefer explicit relationship type from the
value
+ String appropriateRelType =
AtlasEntityUtil.getRelationshipType(attrValue);
+ AtlasAttribute attribute =
entityType.getRelationshipAttribute(attrName, appropriateRelType);
+ mapAttribute(attribute, attrValue, vertex, op, context);
+ }
+
+ LOG.debug("<== mapRelationshipAttributeWithMultipleTypes({}, {})",
attrName, entity.getTypeName());
+ }
+
+ private Map<String, List<Object>>
groupElementsByRelationshipType(Collection<?> relatedObjects,
+ String attrName,
+ Set<String> relationshipTypeNames) {
+ Map<String, List<Object>> elementsByRelationshipType = new HashMap<>();
+
+ // Group related objects by their appropriate relationship type
+ for (Object element : relatedObjects) {
+ // Prefer the explicit relationship type encoded in the element,
if any
+ String appropriateRelType =
AtlasEntityUtil.getRelationshipType(element);
+
+ if (StringUtils.isEmpty(appropriateRelType)) {
Review Comment:
Simplified loop:
Resolve relationshipType per element; if empty and only one configured type,
set it to that type; then continue if still empty or not in the allowed set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
