neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
264 stars 221 forks source link

BUG: Ordering and performance of FlowQuery children operation `q(node).children()` #4507

Open mhsdesign opened 1 year ago

mhsdesign commented 1 year ago

... when using multiple instance of filters:

items = ${q(node).children('[instanceof Vendor.Site:Content.Image],[instanceof Vendor.Site:Content.Text]')}

I would expect the returned nodes to be ordered by index. But this is not the case, nodes with higher index might be returned first, depending on the order of the instanceof filters.

A sorted behavior based on the index is imo rather expected, as its the default behavior for an unfiltered children query and the nodes show up in this order in the ui. The ordering works, because the children operation asks the nodeData repository which internally filters the nodes by index:

https://github.com/neos/neos-development-collection/blob/072186e1601d82a8565942a05c9fe7253cf7f85d/Neos.ContentRepository/Classes/Domain/Repository/NodeDataRepository.php#L884-L903

This sorting is not leveraged by children as we actually iterate one by one over the filters and query the nodeData repository again and again and merge their output:

https://github.com/neos/neos-development-collection/blob/928cc4aa5b427b84cf18ae2af8578e916d0d3afb/Neos.ContentRepository/Classes/Eel/FlowQueryOperations/ChildrenOperation.php#L112

so this flowquery:

${q(node).children('[instanceof Vendor.Site:Content.Image],[instanceof Vendor.Site:Content.Text]')}

actually doesnt translate to this expected optimized code (fewer sql queries)

$this->nodeDataRepository->findByParentAndNodeTypeInContext($ancestorPath, 'Vendor.Site:Content.Image,Vendor.Site:Content.Text', $context, false);

but rather in several calls, which outputs will be merged (but not sorted)

$first = $this->nodeDataRepository->findByParentAndNodeTypeInContext($ancestorPath, 'Vendor.Site:Content.Image', $context, false);
$second = $this->nodeDataRepository->findByParentAndNodeTypeInContext($ancestorPath, 'Vendor.Site:Content.Text', $context, false);

return array_merge($first, $second);

(im using here nodeDataRepository in the example, you might be more familiar with the alias ->getChildNodes oder ->findChildNodes)

This is an issue with Neos ? ... 7.3 ... 8.3 and possibly also Neos 9

mhsdesign commented 1 year ago

Also it might happen that the children operation returns a node twice, because the deduplication logic was not correctly implemented here:

https://github.com/neos/neos-development-collection/blob/928cc4aa5b427b84cf18ae2af8578e916d0d3afb/Neos.ContentRepository/Classes/Eel/FlowQueryOperations/ChildrenOperation.php#L181

($outputNodeAggregateIdentifiers is never written to - since ever)