Skip to content

Proper reversal of relations in sub queries - #171

Open
nilmerg wants to merge 4 commits into
mainfrom
fix/proper-reversal-of-relations-in-sub-queries-170
Open

Proper reversal of relations in sub queries#171
nilmerg wants to merge 4 commits into
mainfrom
fix/proper-reversal-of-relations-in-sub-queries-170

Conversation

@nilmerg

@nilmerg nilmerg commented Aug 20, 2026

Copy link
Copy Markdown
Member

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.

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.

fixes #170

--

A substantial preparation for this is the introduction of Relation::bindTo(Model, string, Resolver) in order to pass control to relations how they're prepared. Since the introduction of BelongsToMany, it is established that a relation may resolve to multiple hops and thus needs to perform steps n-times rather than a single time. It's this reason because registering the alias and resolving the filter is now a responsibility of a relation rather than the resolver.

Relations know it better how to and the override of bindTo in BelongsToMany proves it as it turned out that it is necessary to allow referencing the junction table in either the filter or the through filter in order to be able to better reverse relations.

Qualification must be done by a relation in turn as well, as otherwise there's a mis-match with what's allowed to reference and what can be qualified. My initial attempt was to teach Relation::resolve() this, but without passing it the resolver and changing the return value this doesn't make sense. Sadly, this is out of the question as this is a breaking change. Say hello to Relation::setFilterSubjects() due to this.

@nilmerg nilmerg added this to the v1.0.0 milestone Aug 20, 2026
@nilmerg nilmerg self-assigned this Aug 20, 2026
@cla-bot cla-bot Bot added the cla/signed label Aug 20, 2026
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch 6 times, most recently from f6504d3 to bdcf355 Compare August 25, 2026 15:42
This is required to establish type symmetry as the base's
methods also accept `NULL` to be able to direcly pass a
getters return value to the appropriate setter.
There is now `Relation::bindTo(Model, string, Resolver)`
in order to pass control to relations how they're
prepared. Since the introduction of `BelongsToMany`,
it is established that a relation may resolve to
multiple hops and thus needs to perform steps
n-times rather than a single time. It's this reason
because registering the alias and resolving the filter
is now a responsibility of a relation rather than
the resolver.

Relations know it better how to and the override of
`bindTo` in `BelongsToMany` proves it as it turned
out that it is necessary to allow referencing the
junction table in either the filter or the through
filter in order to be able to better reverse relations.

Qualification must be done by a relation in turn as well,
as otherwise there's a mis-match with what's allowed to
reference and what can be qualified. My initial attempt
was to teach `Relation::resolve()` this, but without
passing it the resolver and changing the return value
this doesn't make sense. Sadly, this is out of the
question as this is a breaking change. Say hello to
`Relation::setFilterSubjects()` due to this.
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.
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.

fixes #170
@nilmerg
nilmerg force-pushed the fix/proper-reversal-of-relations-in-sub-queries-170 branch from bdcf355 to 177dcc4 Compare August 26, 2026 11:03
@nilmerg
nilmerg marked this pull request as ready for review August 26, 2026 11:46
@nilmerg

nilmerg commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

@BastianLedererIcinga Please note that this must not break anything in other products. Icinga DB Web for example must still work without Icinga/icingadb-web#1398, albeit with a deprecation notice here and there.

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.

Query::createSubQuery() attemtps to call BelongsTo::getThroughFilter()

1 participant