Possible regression: Lazy loading deep relations that are soft deleted

[Patrick Spensieri]

Hello Ebean team 👋

We've encountered a change in behaviour when upgrading from 12.12.3 to 13.8.0 related to lazy loading deep relations that are soft-deleted.

I've created a minimal reproduction of the issue, see this pull request for all details.

- The unit tests demonstrate the behaviour with 13.8.0

- If we roll back to 12.12.3, deep relations to soft-deleted entities are lazy-loaded, which is not the case in 13.8.0

- Includes relevant trace-level logs

Let me know how I can help!

Thanks,
Patrick

[Patrick Spensieri]

- The change in behaviour was introduced with version 13.6.6.

- Looks like the change in behaviour is expected given #2750: Stateless update populates OneToMany with softdeleted records during serializing?

[Rob Bygrave]

And you are looking to use eager loading here to get the result expected or thinking the behavior is incorrect ? 

Do you think the fix / change as per 2750 is incorrect or maybe still pondering that question ?

[Patrick Spensieri]

I'm thinking this is a regression in lazy loading because

- With 13.6.6, lazy and eager loading now return different results

- A patch version bump has changed how fetchLazy behaves in a way that can cause unexpected issues

I'll review #2750 more closely, I'm not sure

[Rob Bygrave]

As I see it currently this looks like a case of:

Q: How does includeSoftDeletes propagate wrt lazy loading?

With the status that we have 2 incompatible cases and the suggestion is we can't satisfy both, and so we need to choose which was actually the correct behaviour (as only 1 can be satisfied).

- Was the pre 13.6.6 behaviour correct and we have a regression which means that #2750 wasn't actually a bug and we should not have "fixed" that?

- Or was the pre 13.6.6 behaviour based on a bug ... and fixing the bug wasn't cool (change in behaviour) but in retrospect was still the correct thing to do

- Or are these 2 cases compatible (it doesn't look like they are but ... )

----

Q: How does includeSoftDeletes propagate wrt lazy loading?

We are really looking at that "first lazy loading" that occurs [beyond what the query specified as being fetched].

It's almost conceptual, when we build a graph is the default to includeSoftDelete?  Not really, the default is softDeleted rows are not included unless we explicitly want them.  Once we do that via query.setIncludeSoftDeletes() how far do we expect that to propagate?  Certainly, the current behaviour gives us control over how far that it propagates because we effectively define that via the query.fetch() ... so it seems good to have that control (each use case can define how far the includeSoftDelete propagates).

These are the current thoughts. Hmmm.

--

---
You received this message because you are subscribed to the Google Groups "Ebean ORM" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ebean+un...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ebean/19a36541-a64b-4d9c-9ea7-5a1e37789553n%40googlegroups.com.

[Patrick Spensieri]

Hey Rob, thanks for clear summary.

- I've discussed with the team, and we think the change introduced with #2750 is correct, that is, soft-deleted relations should not be persisted on create

- Like you mentioned, we can use .fetch() or .fetchLazy() to control how far the setIncludeSoftDeletes() should propagate

I've noticed one change in how .fetchLazy() behaves, I'd like to run it by you

- Prior to 13.6.6, query.fetchLazy("account").fetchLazy("account.environment") would return all accounts and soft-deleted environments

- With 13.6.6, this no longer loads all properties in soft-deleted environments

- However, query.fetchLazy("account.environment") works without issue (loads all properties in account, and all properties in soft-deleted environments)

- Note that eager fetching does not have issues when multiple fetch paths are used

I've reproduced the issue here, see the fetchLazy_with_multiple_paths_does_not_fetch_deep_relations_that_are_softdeleted test case.

Q: Is this change in behaviour expected given the changes in 13.6.6?

Thanks,
Patrick

[Patrick Spensieri]

Hey Rob,

Following up to review how fetchLazy() handles multiple fetch paths, which differs from
- Versions prior to 13.6.6

- and from fetch(), which doesn't have issues with multiple fetch paths like query.fetch("account").fetch("account.environment")

Full context in the message above, along with a test case to demonstrate the differences!

Patrick

[Rob Bygrave]

Yes cool. We should create a issue in github for this so that we don't drop the ball on this one. 

To view this discussion on the web visit https://groups.google.com/d/msgid/ebean/9e94e106-2787-43bb-8037-7b6b6e157267n%40googlegroups.com.

[Patrick Spensieri]

Great, I'll post an update in this thread as soon as I do.

[Patrick Spensieri]

Created this issue and documented the workarounds, let me know if you prefer I reproduce the issue in our ebean repo (rather than in the minimal reproduction) 👍