BUGFIX: Improve performance on find*Aggregates

[dlubitz]

The queries to determine the child or parent aggregates have been joined the hierarchy relation (hr) twice. Once for the parent and once for the children. But the hr does contain already both anchorpoints, so a second join is not needed.

Replay without this change:

Replaying events for projection "doctrineDbalContentGraph" of Content Repository "default" ...
 60440/60440 [============================] 100% 13 mins, 59 secs/13 mins, 59 secs 52.0 MiB

Replay with this change:

Replaying events for projection "doctrineDbalContentGraph" of Content Repository "default" ...
 60440/60440 [============================] 100%  4 mins, 13 secs/4 mins, 13 secs  52.0 MiB

Fixes: #5269

[mhsdesign]

Sadly there are no test for findParentNodeAggregates and maybe the other find*Aggregates so the ci passing might not be a reliable indicator. Id say we need tests here as well ._.

[dlubitz]

Sadly there are no test for findParentNodeAggregates and maybe the other find*Aggregates so the ci passing might not be a reliable indicator. Id say we need tests here as well ._.

But they are tested within the behat tests. If you mess up there, a lot of tests are failing.

[skurfuerst]

I think this is correct, but I'd love @nezaniel to have a second look over it :)

[bwaidelich]

Amazing, thank you!

[mhsdesign]

It seems we found the culprit. The reason for this obsolete join is history. Originally the nodename was stored in the hierarchy relation table. And with #5018 as stated:

Node names are now stored in the contentgraph's node table

The node name is now in the node table.

There are actually more hidden places that can seemingly be optimised we found at least 4 places where a join was annotated being made for the legacy reason:

// we need to join with the hierarchy relation, because we need the node name.

[dlubitz]

This seems to be a bit more complex than expected. I need to check if the tests are covering all cases and if the changes still make sence.

[dlubitz]

I've splitted the changes. This PR now only contains the first change I did. This only affects the NodeAggregates query, which we can safely improve, as they don't join subtree tags.

The other changes are now in a draft PR #5273. I need to find a way to test these queries, especially the resulting subtree tags. As it makes a difference for subtree tags if I use the parentanchorpoint or the childanchorpoint to join the node data.

[mhsdesign]

fyi the only difference to the previously used buildBasicNodeAggregateQuery is that we join on h.parentnodeanchor instead of h.childnodeanchor

I had the feeling introducing this might deduplicate code another place and it could:

In findParentNodeAggregateByChildOriginDimensionSpacePoint we use the same logic besides one additional where. So i think it could be nicely deduplicated like:

public function findParentNodeAggregateByChildOriginDimensionSpacePoint(NodeAggregateId $childNodeAggregateId, OriginDimensionSpacePoint $childOriginDimensionSpacePoint): ?NodeAggregate
    {
        $subQueryBuilder = ...;
    
        $queryBuilder = $this->nodeQueryBuilder->buildParentNodeAggregateQuery()
            ->andWhere('n.nodeaggregateid = (' . $subQueryBuilder->getSQL() . ')')
            ->setParameters([
                'contentStreamId' => $this->contentStreamId->value,
                'childNodeAggregateId' => $childNodeAggregateId->value,
                'childOriginDimensionSpacePointHash' => $childOriginDimensionSpacePoint->hash,
            ]);

Also i dont exactly understand how this utility is supposed to work (when do we pass arguments or when to use setParameters) but because we dont return a final query i think its buildBasicParentNodeAggregateQuery

[dlubitz]

Is your comment only for clarification? Or do you complain about the partially duplication of the queries and want to get that changed?

[mhsdesign]

i was just thoroughly reviewing this and partly investigating and trying to understand. So i guess 50/50 ill create a followup pr :D

_> #5276

[mhsdesign]

By reading, thanks for the effort and also trying to improve the other queries (after me sidetracking you into that idea which turned out to be more complex^^)