Skip to content

Commit cfad3ff

Browse files
aklishAaron KlishWilliam Cekan
authored
Fixed N+1 problems for all toOne relationships (#1241)
* Fixed N+1 problems for all toOne relationships * Skip fetch join on parent object when fetching a relationship * Fixed checkstyle Co-authored-by: Aaron Klish <[email protected]> Co-authored-by: William Cekan <[email protected]>
1 parent 57bd29c commit cfad3ff

File tree

6 files changed

+78
-21
lines changed

6 files changed

+78
-21
lines changed

‎elide-datastore/elide-datastore-hibernate/src/main/java/com/yahoo/elide/core/hibernate/hql/AbstractHQLQueryBuilder.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,9 @@
2525
import java.util.Map;
2626
import java.util.Optional;
2727
import java.util.Set;
28+
import java.util.function.Function;
2829
import java.util.stream.Collectors;
2930

30-
import javax.persistence.OneToOne;
31-
3231
/**
3332
* Abstract class used to construct HQL queries.
3433
*/
@@ -190,15 +189,17 @@ private String extractJoinClause(FilterPredicate predicate, Set<String> alreadyJ
190189
* @return The JOIN clause that can be added to the FROM clause.
191190
*/
192191
protected String extractToOneMergeJoins(Class<?> entityClass, String alias) {
192+
return extractToOneMergeJoins(entityClass, alias, (unused) -> false);
193+
}
194+
195+
protected String extractToOneMergeJoins(Class<?> entityClass, String alias,
196+
Function<String, Boolean> skipRelation) {
193197
List<String> relationshipNames = dictionary.getRelationships(entityClass);
194198
StringBuilder joinString = new StringBuilder("");
195199
for (String relationshipName : relationshipNames) {
196200
RelationshipType type = dictionary.getRelationshipType(entityClass, relationshipName);
197201
if (type.isToOne() && !type.isComputed()) {
198-
// fetch only OneToOne with mappedBy
199-
OneToOne oneToOne = dictionary.getAttributeOrRelationAnnotation(
200-
entityClass, OneToOne.class, relationshipName);
201-
if (oneToOne == null || oneToOne.mappedBy().isEmpty()) {
202+
if (skipRelation.apply(relationshipName)) {
202203
continue;
203204
}
204205
joinString.append(" LEFT JOIN FETCH ");

‎elide-datastore/elide-datastore-hibernate/src/main/java/com/yahoo/elide/core/hibernate/hql/SubCollectionFetchQueryBuilder.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.yahoo.elide.core.hibernate.Session;
1414

1515
import java.util.Collection;
16+
import java.util.function.Function;
1617

1718
/**
1819
* Constructs a HQL query to fetch a hibernate collection proxy.
@@ -28,6 +29,24 @@ public SubCollectionFetchQueryBuilder(Relationship relationship,
2829
this.relationship = relationship;
2930
}
3031

32+
@Override
33+
protected String extractToOneMergeJoins(Class<?> entityClass, String alias) {
34+
Function<String, Boolean> shouldSkip = (relationshipName) -> {
35+
String inverseRelationName = dictionary.getRelationInverse(entityClass, relationshipName);
36+
if (inverseRelationName.isEmpty()) {
37+
return false;
38+
}
39+
40+
Class<?> relationshipClass = dictionary.getParameterizedType(entityClass, relationshipName);
41+
42+
//We don't need (or want) to fetch join the parent object.
43+
return relationshipClass.equals(relationship.getParentType())
44+
&& inverseRelationName.equals(relationship.getRelationshipName());
45+
};
46+
47+
return extractToOneMergeJoins(entityClass, alias, shouldSkip);
48+
}
49+
3150
/**
3251
* Constructs a query that returns the members of a relationship.
3352
*

‎elide-datastore/elide-datastore-hibernate/src/test/java/com/yahoo/elide/datastores/hibernate/hql/AbstractHQLQueryBuilderTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ public void testFilterJoinClause() {
108108
public void testFetchJoinClause() {
109109
String actual = extractToOneMergeJoins(Left.class, "right_alias");
110110

111-
String expected = " LEFT JOIN FETCH right_alias.noUpdateOne2One LEFT JOIN FETCH right_alias.one2one ";
111+
String expected = " LEFT JOIN FETCH right_alias.noDeleteOne2One "
112+
+ "LEFT JOIN FETCH right_alias.noUpdateOne2One "
113+
+ "LEFT JOIN FETCH right_alias.one2one ";
112114
assertEquals(expected, actual);
113115
}
114116

‎elide-datastore/elide-datastore-hibernate/src/test/java/com/yahoo/elide/datastores/hibernate/hql/RootCollectionFetchQueryBuilderTest.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ public void testRootFetch() {
5454

5555
TestQueryWrapper query = (TestQueryWrapper) builder.build();
5656

57-
String expected = "SELECT example_Book FROM example.Book AS example_Book ";
57+
String expected =
58+
"SELECT example_Book FROM example.Book AS example_Book LEFT JOIN FETCH example_Book.publisher ";
5859
String actual = query.getQueryText();
5960

6061
assertEquals(expected, actual);
@@ -72,7 +73,8 @@ public void testRootFetchWithSorting() {
7273
.withPossibleSorting(Optional.of(new Sorting(sorting)))
7374
.build();
7475

75-
String expected = "SELECT example_Book FROM example.Book AS example_Book order by example_Book.title asc";
76+
String expected = "SELECT example_Book FROM example.Book AS example_Book "
77+
+ "LEFT JOIN FETCH example_Book.publisher order by example_Book.title asc";
7678
String actual = query.getQueryText();
7779

7880
assertEquals(expected, actual);
@@ -145,8 +147,8 @@ public void testRootFetchWithSortingAndFilters() {
145147
.build();
146148

147149
String expected =
148-
"SELECT example_Book FROM example.Book AS example_Book "
149-
+ "WHERE example_Book.id IN (:id_XXX) order by example_Book.title asc";
150+
"SELECT example_Book FROM example.Book AS example_Book LEFT JOIN FETCH example_Book.publisher"
151+
+ " WHERE example_Book.id IN (:id_XXX) order by example_Book.title asc";
150152

151153
String actual = query.getQueryText();
152154
actual = actual.replaceFirst(":id_\\w+", ":id_XXX");

‎elide-datastore/elide-datastore-hibernate/src/test/java/com/yahoo/elide/datastores/hibernate/hql/SubCollectionFetchQueryBuilderTest.java

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.yahoo.elide.core.filter.InPredicate;
1515
import com.yahoo.elide.core.hibernate.hql.RelationshipImpl;
1616
import com.yahoo.elide.core.hibernate.hql.SubCollectionFetchQueryBuilder;
17+
import com.yahoo.elide.core.pagination.Pagination;
1718
import com.yahoo.elide.core.sort.Sorting;
1819

1920
import example.Author;
@@ -100,7 +101,7 @@ public void testSubCollectionFetchWithSorting() {
100101
.build();
101102

102103
String expected = "SELECT example_Book FROM example.Author example_Author__fetch "
103-
+ "JOIN example_Author__fetch.books example_Book "
104+
+ "JOIN example_Author__fetch.books example_Book LEFT JOIN FETCH example_Book.publisher "
104105
+ "WHERE example_Author__fetch=:example_Author__fetch order by example_Book.title asc";
105106
String actual = query.getQueryText();
106107

@@ -141,7 +142,7 @@ public void testSubCollectionFetchWithJoinFilter() {
141142

142143
String expected = "SELECT example_Book FROM example.Author example_Author__fetch "
143144
+ "JOIN example_Author__fetch.books example_Book "
144-
+ "LEFT JOIN example_Book.publisher example_Book_publisher "
145+
+ "LEFT JOIN example_Book.publisher example_Book_publisher LEFT JOIN FETCH example_Book.publisher "
145146
+ "WHERE example_Book_publisher.name IN (:books_publisher_name_XXX) AND example_Author__fetch=:example_Author__fetch ";
146147
String actual = query.getQueryText();
147148
actual = actual.replaceFirst(":publisher_name_\\w+_\\w+", ":books_publisher_name_XXX");
@@ -187,12 +188,48 @@ public void testSubCollectionFetchWithSortingAndFilters() {
187188

188189
String expected = "SELECT example_Book FROM example.Author example_Author__fetch "
189190
+ "JOIN example_Author__fetch.books example_Book "
190-
+ "LEFT JOIN example_Book.publisher example_Book_publisher "
191+
+ "LEFT JOIN example_Book.publisher example_Book_publisher LEFT JOIN FETCH example_Book.publisher "
191192
+ "WHERE example_Book_publisher.name IN (:publisher_name_XXX) AND example_Author__fetch=:example_Author__fetch order by example_Book.title asc";
192193

193194
String actual = query.getQueryText();
194195
actual = actual.replaceFirst(":publisher_name_\\w+", ":publisher_name_XXX");
195196

196197
assertEquals(expected, actual);
197198
}
199+
200+
@Test
201+
public void testFetchJoinExcludesParent() {
202+
Publisher publisher = new Publisher();
203+
publisher.setId(1);
204+
205+
Book book = new Book();
206+
book.setId(2);
207+
208+
RelationshipImpl relationship = new RelationshipImpl(
209+
Publisher.class,
210+
Book.class,
211+
BOOKS,
212+
publisher,
213+
Arrays.asList(book)
214+
);
215+
216+
SubCollectionFetchQueryBuilder builder = new SubCollectionFetchQueryBuilder(
217+
relationship, dictionary, new TestSessionWrapper());
218+
219+
TestQueryWrapper query = (TestQueryWrapper) builder
220+
.withPossibleFilterExpression(Optional.empty())
221+
.withPossibleSorting(Optional.empty())
222+
.withPossiblePagination(
223+
Optional.of(Pagination.fromOffsetAndLimit(1, 1, false)))
224+
.build();
225+
226+
String expected = "SELECT example_Book FROM example.Publisher example_Publisher__fetch "
227+
+ "JOIN example_Publisher__fetch.books example_Book "
228+
+ "WHERE example_Publisher__fetch=:example_Publisher__fetch";
229+
230+
String actual = query.getQueryText();
231+
actual = actual.replaceFirst(":publisher_name_\\w+", ":publisher_name_XXX");
232+
233+
assertEquals(expected, actual);
234+
}
198235
}

‎elide-integration-tests/src/test/java/com/yahoo/elide/tests/SortingIT.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,7 @@ public void setup() {
5757
@Test
5858
public void testSortingRootCollectionByRelationshipProperty() throws IOException {
5959
JsonNode result = getAsNode("/book?sort=-publisher.name");
60-
//We expect 2 results because the Hibernate does an inner join between book & publisher
61-
assertEquals(2, result.get("data").size());
62-
60+
int size = result.get("data").size();
6361

6462
JsonNode books = result.get("data");
6563
String firstBookName = books.get(0).get("attributes").get("title").asText();
@@ -69,14 +67,12 @@ public void testSortingRootCollectionByRelationshipProperty() throws IOException
6967
assertEquals("The Old Man and the Sea", secondBookName);
7068

7169
result = getAsNode("/book?sort=publisher.name");
72-
//We expect 2 results because the Hibernate does an inner join between book & publisher
73-
assertEquals(2, result.get("data").size());
7470

7571
books = result.get("data");
76-
firstBookName = books.get(0).get("attributes").get("title").asText();
72+
firstBookName = books.get(size - 2).get("attributes").get("title").asText();
7773
assertEquals("The Old Man and the Sea", firstBookName);
7874

79-
secondBookName = books.get(1).get("attributes").get("title").asText();
75+
secondBookName = books.get(size - 1).get("attributes").get("title").asText();
8076
assertEquals("For Whom the Bell Tolls", secondBookName);
8177
}
8278

0 commit comments

Comments
 (0)