Skip to content

Query: Always use INNER join to establish the outer query link - #173

Draft
nilmerg wants to merge 3 commits into
fix/proper-reversal-of-relations-in-sub-queries-170from
always-use-inner-joins-for-sub-queries
Draft

nilmerg wants to merge 3 commits into
fix/proper-reversal-of-relations-in-sub-queries-170from
always-use-inner-joins-for-sub-queries

Conversation

@nilmerg

@nilmerg nilmerg commented Aug 26, 2026

Copy link
Copy Markdown
Member

A subquery always needs to establish a link, be it part of a WHERE or solely used to eager load (using Query::derive) a relation. Using an INNER join allows the database to apply optimizations it wouldn't be able to otherwise.

Since this affects Query::derive(), any eager loaded relation will now return no result. But this should be usually not a problem, as a caller that iterates or calls first() without handling the never entered loop or null-return is already bugged, IMHO. I've checked this with Icinga DB Web and found exactly two cases: Icinga/icingadb-web#1408

Still attempting to prove this is actually of advantage, though…

@nilmerg nilmerg added this to the v1.0.0 milestone Aug 26, 2026
@nilmerg nilmerg self-assigned this Aug 26, 2026
@cla-bot cla-bot Bot added the cla/signed label Aug 26, 2026
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch from 177dcc4 to bcd7fd8 Compare September 2, 2026 08:52
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch 2 times, most recently from e5a190f to ab54983 Compare September 21, 2026 13:46
Changes the way relations can be reversed drastically
as it is now possible to influence the relation to use
during reversal with `::setReverseName(string)` which
allows Icinga DB Web to drop the error-prone `to.from`
and `from.to` relations. An additional change is that
it is now not mandatory anymore to define relations
that are solely being required because of sub-queries.
Missing relations on the reversed path are automatically
registered. For this, each relation type now has its
specific counterpart which is possible to override
with `::setReverseClass(class-string)`. The default
however, is to use the same type which is the case
for `BelongsToOne` and `BelongsToMany`. For `BelongsTo`
a sane override has been chosen that is based on how
it's used at the moment in our products, as `HasOne`
and `HasMany` may both be appropriate. But the latter
clearly is used more often.
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch from d3a89ab to c10c699 Compare September 21, 2026 14:23
@nilmerg
nilmerg force-pushed the always-use-inner-joins-for-sub-queries branch from a9e6101 to f8337a9 Compare September 21, 2026 14:29
Since `::reverse()` uses the source's table alias by default
as reverse name, a deprecation notice is triggered if the
original forward path uses a different name, indicating that
it is necessary to use this name as explicit reverse name.

As `Query::createSubQuery()` now overrides the source's
relations, `Query::derive()` cannot use it anymore as
otherwise the result will not have the relations it had
previously. This must be done in order to avoid breaking
current usages that rely on specific relations for filters
or columns. A side-effect of this is, that the resulting
query does not use the `sub_` alias prefix anymore.

fixes #170
A subquery always needs to establish a link, be it part of a
WHERE or solely used to eager load (using `Query::derive`)
a relation. Using an INNER join allows the database to apply
optimizations it wouldn't be able to otherwise.
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch from c10c699 to cc5ee0c Compare September 21, 2026 14:51
@nilmerg
nilmerg force-pushed the always-use-inner-joins-for-sub-queries branch from f8337a9 to 8985dfa Compare September 21, 2026 14:51
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch from 7b2c3ab to 3cc4ef0 Compare September 22, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant