Skip to content

Commit 46a35d0

Browse files
authored
Fix multiple relation batch update (#22352)
* Fix multiple relationship bulk add * Remove unused code
1 parent b48a02d commit 46a35d0

2 files changed

Lines changed: 76 additions & 23 deletions

File tree

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4222,30 +4222,45 @@ public final void updateToRelationships(
42224222

42234223
// Batch add new relationships
42244224
if (!added.isEmpty()) {
4225-
// Create forward relationships
4226-
List<UUID> addedIds =
4227-
added.stream().map(EntityReference::getId).collect(Collectors.toList());
4228-
daoCollection
4229-
.relationshipDAO()
4230-
.bulkInsertToRelationship(
4231-
fromId, addedIds, fromEntityType, toEntityType, relationshipType.ordinal());
4232-
42334225
if (bidirectional) {
4234-
// Create reverse relationships using bulkInsertTo for true batch operation
4235-
List<CollectionDAO.EntityRelationshipObject> reverseRelationships =
4236-
added.stream()
4237-
.map(
4238-
ref ->
4239-
CollectionDAO.EntityRelationshipObject.builder()
4240-
.fromId(ref.getId().toString())
4241-
.toId(fromId.toString())
4242-
.fromEntity(ref.getType())
4243-
.toEntity(fromEntityType)
4244-
.relation(relationshipType.ordinal())
4245-
.build())
4246-
.collect(Collectors.toList());
4247-
4248-
daoCollection.relationshipDAO().bulkInsertTo(reverseRelationships);
4226+
// For bidirectional relationships, apply the optimization where smaller UUID is always
4227+
// fromId
4228+
List<CollectionDAO.EntityRelationshipObject> optimizedRelationships = new ArrayList<>();
4229+
for (EntityReference ref : added) {
4230+
UUID id1 = fromId;
4231+
UUID id2 = ref.getId();
4232+
String entity1 = fromEntityType;
4233+
String entity2 = ref.getType();
4234+
4235+
// Ensure smaller UUID is always fromId for bidirectional relationships
4236+
if (id1.compareTo(id2) > 0) {
4237+
// Swap the IDs and entities
4238+
UUID tempId = id1;
4239+
id1 = id2;
4240+
id2 = tempId;
4241+
String tempEntity = entity1;
4242+
entity1 = entity2;
4243+
entity2 = tempEntity;
4244+
}
4245+
4246+
optimizedRelationships.add(
4247+
CollectionDAO.EntityRelationshipObject.builder()
4248+
.fromId(id1.toString())
4249+
.toId(id2.toString())
4250+
.fromEntity(entity1)
4251+
.toEntity(entity2)
4252+
.relation(relationshipType.ordinal())
4253+
.build());
4254+
}
4255+
daoCollection.relationshipDAO().bulkInsertTo(optimizedRelationships);
4256+
} else {
4257+
// For non-bidirectional relationships, just create forward relationships
4258+
List<UUID> addedIds =
4259+
added.stream().map(EntityReference::getId).collect(Collectors.toList());
4260+
daoCollection
4261+
.relationshipDAO()
4262+
.bulkInsertToRelationship(
4263+
fromId, addedIds, fromEntityType, toEntityType, relationshipType.ordinal());
42494264
}
42504265
}
42514266
if (!nullOrEmpty(updatedToRefs)) {

openmetadata-service/src/test/java/org/openmetadata/service/resources/metrics/MetricResourceTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,43 @@ public void setupMetrics() throws IOException {
5757
Metric2 = createEntity(createMetric, ADMIN_AUTH_HEADERS);
5858
}
5959

60+
@Test
61+
void test_duplicateRelatedMetricsIssue() throws IOException {
62+
CreateMetric createMetric1 = createRequest("test_metric_duplicate_1", "", "", null);
63+
Metric metric1 = createEntity(createMetric1, ADMIN_AUTH_HEADERS);
64+
65+
CreateMetric createMetric2 = createRequest("test_metric_duplicate_2", "", "", null);
66+
Metric metric2 = createEntity(createMetric2, ADMIN_AUTH_HEADERS);
67+
68+
Metric originalMetric2 = getMetric(metric2.getId(), "*", ADMIN_AUTH_HEADERS);
69+
String origJson = JsonUtils.pojoToJson(originalMetric2);
70+
71+
originalMetric2.setRelatedMetrics(List.of(metric1.getEntityReference()));
72+
patchEntity(originalMetric2.getId(), origJson, originalMetric2, ADMIN_AUTH_HEADERS);
73+
74+
Metric updatedMetric2 = getMetric(metric2.getId(), "relatedMetrics", ADMIN_AUTH_HEADERS);
75+
76+
Assertions.assertNotNull(updatedMetric2.getRelatedMetrics());
77+
Assertions.assertEquals(
78+
1,
79+
updatedMetric2.getRelatedMetrics().size(),
80+
"Expected only 1 related metric, but found "
81+
+ updatedMetric2.getRelatedMetrics().size()
82+
+ ". Related metrics: "
83+
+ updatedMetric2.getRelatedMetrics());
84+
Assertions.assertEquals(metric1.getId(), updatedMetric2.getRelatedMetrics().getFirst().getId());
85+
86+
// Also verify that metric1 now has metric2 as a related metric (bidirectional)
87+
Metric updatedMetric1 = getMetric(metric1.getId(), "relatedMetrics", ADMIN_AUTH_HEADERS);
88+
Assertions.assertNotNull(updatedMetric1.getRelatedMetrics());
89+
Assertions.assertEquals(
90+
1,
91+
updatedMetric1.getRelatedMetrics().size(),
92+
"Expected only 1 related metric for the reverse relationship, but found "
93+
+ updatedMetric1.getRelatedMetrics().size());
94+
Assertions.assertEquals(metric2.getId(), updatedMetric1.getRelatedMetrics().getFirst().getId());
95+
}
96+
6097
@Test
6198
void patch_MetricEntity() throws IOException {
6299
// Create a new Metric with different fields
@@ -177,6 +214,7 @@ public Metric validateGetWithDifferentFields(Metric entity, boolean byName)
177214
return entity;
178215
}
179216

217+
@SuppressWarnings("unchecked")
180218
@Override
181219
public void assertFieldChange(String fieldName, Object expected, Object actual) {
182220
if (expected != null && actual != null) {

0 commit comments

Comments
 (0)