Relationships are loaded without data consistency via separate queries instead of JOINs

[leio]

What you did

A belongs_to relationship is loaded with a separate SQL SELECT when either eager-loaded or used with .include() if toasty::Deferred wrapped in a BEGIN/COMMIT block on at least PostgreSQL.

So querying a single (child) table row via get_by_id() with the parent relationship included is issuing something like this to the database:

BEGIN;
SELECT
  tbl_0_0."id" AS column1,
  tbl_0_0."parent_id" AS column2,
  tbl_0_0."name" AS column3
FROM "child_table" AS tbl_0_0
WHERE tbl_0_0."id" = $1;
SELECT
  tbl_0_0."id" AS column1,
  tbl_0_0."parent_name" AS column2
FROM "parent_table" AS tbl_0_0
WHERE EXISTS (SELECT $2 AS column1 FROM (VALUES ($1)) AS tbl_1_0 WHERE tbl_1_0.column1 = tbl_0_0."id");
COMMIT;

This doesn't actually guarantee data consistency because the SELECTs happen at a different time. Transactions with the default isolation level does not make the separate SELECTs see the data from the same point of time - if something else updates data (and commits it if using explicit transactions) inbetween the separate SELECT calls issued here database connection roundtrips, then the data will not be consistent. This is very bad in my book.

The correct way to do this is to JOIN (the appropriate type, e.g. LEFT JOIN if the relationship is nullable or that just in case) or some other CTE thing as to do this in a single statement.

What you expected

I expected the ORM to issue a query more like this:

SELECT
  tbl_0_0."id" AS column1,
  tbl_0_0."parent_id" AS column2,
  tbl_0_0."name" AS column3,
  tbl_1_0."id" AS column4,
  tbl_1_0."parent_name" AS column5
FROM "child_table" AS tbl_0_0
LEFT JOIN "parent_table" AS tbl_1_0
ON tbl_1_0."id" = tbl_0_0."parent_id"
WHERE tbl_0_0."id" = $1;

This is safe in terms of data consistency, so we won't at least have the child and parent models in application logic seen from a different point of time.

What actually happened

No response

Minimal reproducer

Database driver

PostgreSQL

Toasty version

0.9.0

Additional context

No response

[fredrikroos]

The correct way of doing this is by setting the appropriate isolation level. Select statements are always at least repeatable read within the statement itself, so by stuffing everything into a single select under the default isolation level all you are doing is just raising the isolation level implicitly. You might as well do that explicitly then. This also doesn't force you to use suboptimal queries (the first variant is often more efficient).

[leio]

The correct way of doing this is by setting the appropriate isolation level. Select statements are always at least repeatable read within the statement itself, so by stuffing everything into a single select under the default isolation level all you are doing is just raising the isolation level implicitly. You might as well do that explicitly then. This also doesn't force you to use suboptimal queries (the first variant is often more efficient).

Sorry, but that's not really the case in common real world applications in my experience, which I believe is what Toasty targets.

• Changing the isolation level to REPEATABLE READ or above for everything by default will make it (to my knowledge) impossible to get fresh data of anything after an explicit lock acquiring, all the data will be old from when the transaction started, not what the state of things is after the lock was acquired.
• Single query with LEFT JOIN instead of two like done now is faster by a full database roundtrip time, and this can not be sped up with command pipelining in this form - the second SELECT needs to know the parent_id from the first SELECT. Two queries is highly inefficient, contrary to any actual query optimality, which I highly doubt without proof is even more optimal as separate query statements.
• A LEFT JOIN approach does not temporarily raise the isolation level implicitly, that's a transaction property, not a SQL statement property. Yes, there's sort of a snapshot involved, I believe, but it's all MVCC stuff, and it doesn't affect following SELECT statements, some of which may be required to be able to read committed (by other connections/transactions) data.

Given the roundtrip times, I can not see how a LEFT JOIN would be more suboptimal to two separate queries, from that fact alone. We might talk about that if it involves lots of duplicated parent rows when querying multiple child rows of the same parent, involving many levels of JOINs or something like that.
If that's the case, sure, do a separate eager load query and cause related data inconsistency (as in my experience no-one in the type of applications that would use Toasty would be raising the isolation level for common usage, such as happens when implicitly getting relationships loaded via #[belongs_to]), but it should (eventually) do better for just a single child row query.
And so this issue was filed for consideration.

[carllerche]

I do think that eager-loading cardinality-1 associations probably is best done with a JOIN. I'll take a look to see what that would take.

[fredrikroos]

A LEFT JOIN approach does not temporarily raise the isolation level implicitly

I mean that a select statement raises the isolation level temporarily, by reusing the same snapshot for the duration of a select it will always have repeatable read semantics internally. It essentially executes in an implicit subtransaction at weaker isolation levels. This is why your proposed solution works, it puts all the work inside this implicit transaction so everything behaves as if the transaction is repeatable read.

We might talk about that if it involves lots of duplicated parent rows when querying multiple child rows of the same parent, involving many levels of JOINs or something like that.

Yes this is what I'm referring to, I was talking in general and not specifically about this trivial example. There are also issues with top-n queries and such when you have more than one row per returned entity.

In Postgres you can in theory work around this using array_agg and row variables, but that would potentially lead to very large column values so not sure how it works out in practice.