[nilmerg]

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]

@lippserd Please share you opinion and experience regarding the advantage this might have.

[lippserd]

It's been quite a while since I last benchmarked Icinga DB queries and filter combinations. If I remember correctly, that was against MySQL 5.6 and MySQL 8. Back then, LEFT joins turned out to be more flexible in terms of query plans, since MySQL wasn't forced into an INNER join straight away. But that was for regular joins, not subqueries. So to actually help here, I ran some tests.

Setup: MySQL 8.4 and 9.7, MariaDB 11.8 and 12.3, PostgreSQL 17 and 18, with our devlab data scaled up 300 times (about 31k hosts and 335k services). I generated the queries from Icinga DB Web's models, once with this change and once without it.

The good news first: the results are identical everywhere, so the change is safe.

In most cases it doesn't make a difference, because the databases already turn the LEFT join into an INNER join on their own: as soon as some condition rules out the extra NULL rows of the LEFT join, they know it can't matter, e.g., pk=fk. That covered almost everything I tried on services, hosts, comments and downtimes: host and service groups, custom vars, AND and OR combinations, negations and custom-var columns. Same plans, same timings.

Where it does help: filters combined with OR on the comments and downtimes lists. There, nothing rules out the NULL rows, so MariaDB and PostgreSQL keep the LEFT join. Since those tables have no index on host_id/service_id, they end up being scanned over and over. With your change they don't:

Page query	MariaDB 12.3	PostgreSQL 18	MySQL 8.4
Comments: host.hostgroup.name=…|service.servicegroup.name=…	3.9 s → 14 ms	10 → 3 ms	no change
Comments: host var OR service var with wildcard	99 s → 42 ms	144 → 27 ms	no change
Downtimes: same filter	>120 s → 58 ms	144 → 30 ms	no change

The first filter is an ordinary one, and the same applies when it's used as a restriction. So this is a real improvement for those cases. I also checked whether adding the missing indexes would solve this without your change: they fix MariaDB's worst case, but don't help PostgreSQL, so your change is the better fix.

A nice side effect concerns the IS NOT NULL we add to negated filters. With the LEFT join it's essential: without it, service.vars.location!~*o* on downtimes returns 0 rows instead of 4893, on both MariaDB and PostgreSQL. With INNER it's no longer needed at all. Adding IS NOT NULL to the OR sub queries above (while keeping the LEFT join) gives exactly the same speed-up as your change. That confirms it's really just about telling the database that the NULL rows can't match.

Just one small request: could you relax the commit message a bit? "Allows the database to apply optimizations it wouldn't be able to otherwise" sounds like a general win, while in practice databases mostly do this already and the gain shows up only in cases like the above. Maybe something like:

A sub query always needs to establish a link to the outer query, so rows added by a LEFT join on that link can never match. Using an INNER join makes this explicit instead of relying on the database to figure it out, which it doesn't always do (e.g. MariaDB and PostgreSQL with sub queries combined by OR).

Separately, I noticed a few columns in Icinga DB that relations look up by but that have no index (e.g. comment/downtime host_id/service_id, notification_history host_id/service_id). I'll look into that in a separate PR for Icinga DB.