fix: EXPOSED-217 Unnecessary query after calling with() and iteration #2017
Conversation
When eager loading references from a SizedIterable of entities, with() first iterates over the object then preloads relations, which correctly generates 2 SQL queries. If this eager loading is followed by iteration, the first SQL query is executed again because its results are discarded, while the retrieved references are cached. This fix stores the result of the first query in the SizedIterable instance to ensure that the right iterator is invoked.
| /** Represents the iterable elements of a database result, which are stored once loaded on first access. */ | ||
| interface LazySizedIterable<T> : SizedIterable<T> { | ||
| /** The lazily loaded database result. */ | ||
| var loadedResult: List<T>? | ||
| } | ||
|
|
There was a problem hiding this comment.
An alternative to this would be to add a function to the base SizedIterable interface, something like
fun cacheLoadedResult(result: List<*>) {}that could be overriden in the anonymous object below to set a private variable.
This issue with that would be that cacheLoadedResult() would be available to users for every SizedIterable (which means every single query, as AbstractQuery implements it):
MyTable.selectAll().cacheLoadedResult()
// this would do nothing and I'm not sure what the expected behavior/value should/could be
| assertEquals(1, oneSchool.size) | ||
| // expected: 1 query to select all School, 1 query to select the referenced Regions, | ||
| // then 1 new query to select only first School | ||
| assertEquals(3, statementCount) | ||
| assertEquals(statementCount, statementStats.size) | ||
| statementStats.assertEachQueryExecutedOnlyOnce() | ||
|
|
||
| debug = false |
There was a problem hiding this comment.
The current set up of the anonymous SizedIterable is that it makes new copies of the invoking source iterable (without the cached result) when adjusting the underlying query with chained functions.
So even when a result has been cached, chaining functions afterwards changes the expected query and triggers a new SQL execution on iteration because loadedResult is reset to null.
If it turns out that this is not the expected behavior for users, loadedResult logic can be easily added to the overrides in the anonymous function.
Using
all()to get a collection of entities returns an instance of an anonymousSizedIterableobject.When eager loading references from this object,
with()first iterates over the object then preloads relations for each retrieved entity, which correctly generates 2 SQL statements:If this eager loading is followed by iteration (using
toList()orforEach()e.g.), the first SQL statement is executed again because the anonymous object delegates to the source iterator,AbstractQuery.iteratorand its results (from the first execution) are discarded after being used to load references.The retrieved references, however, are cached in each
Entity.referenceCache, so the second SQL is not executed again:This occurs because
with()was refactored in PR #1585 to return the same invoking type, rather than a List. So the source iterator of the returnedSizedIterableexecutes a query when called.This fix stores the result of the first query in the
SizedIterableinstance and ensures the right iterator is invoked if a result has been cached.