Skip to content

Bugfix/filling gaps and eager loading - #19318

Draft
i-just wants to merge 3 commits into
5.xfrom
bugfix/filling-gaps-and-eager-loading
Draft

Bugfix/filling gaps and eager loading#19318
i-just wants to merge 3 commits into
5.xfrom
bugfix/filling-gaps-and-eager-loading

Conversation

@i-just

@i-just i-just commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

Issues:

  • The number of queries used to retrieve a set of element with Categories field that has maintainHierarchy turned on is different depending on whether you don’t eager-load vs eager-load with .with() vs lazy eager-load with .eagerly() vs lazy eager-load with .eagerly() with alias (.eagerly('myAlias')).
  • What’s more, the results you get also differ between non eager-loaded and any of the eager-loaded queries if there are actual gaps to fill.

Steps to reproduce:

  1. clean craft cms installation
  2. a categories group with all default settings
  3. a categories field with all the default settings
  4. 9 categories in that group, all on the top level
  5. a channel-type section with revisions turned off and with 3 entry types
  6. each of those entry types has a title and the categories field mentioned above
  7. 5 entries in that section, entry 1 uses entry type 1, entry 2 uses entry type 2 and entries 3-5 use entry type 3; the first one has the first 3 categories in the categories field, the second one has the middle 3, and the remaining have the last 3 categories each
  8. following twig template
<!DOCTYPE html>
<html lang="en-US">
<head>
    <meta http-equiv="X-UA-Compatible" content="IE=edge"/>
    <meta charset="utf-8"/>
    <title>Welcome to Craft CMS</title>
</head>
<body class="ltr">
{% set myEntries = craft.entries().section('sect1').type(['et1','et2','et3']).all() %}
{#{% set myEntries = craft.entries().section('sect1').type(['et1','et2','et3']).with(['myCategories']).all() %}#}
{% for myEntry in myEntries %}
    <h2>{{ myEntry.title }}</h2>
{#    {% set myCategories = myEntry.myCategories.eagerly('myAlias').all() %}#}
    {% set myCategories = myEntry.myCategories.all() %}
    <ul>
        {% for myCategory in myCategories %}
            <li>{{ myCategory.title }}</li>
        {% endfor %}
    </ul>
{% endfor %}

</body>
</html>
  1. ensure you have debug toolbar on for your account
  2. access the template in the browser and note the number of queries; uncomment/modify relevant parts to test with, eagerly and eagerly('myAlias') and check the number of db calls for each
  3. edit the categories field and turn on maintainHierarchy
  4. repeat step 10
  5. add another category (title, e.g. "fill me in"), place it under the 7th category (on top level) and then make the 8th category its child
  6. repeat step 10 again

Results before this PR:

--------------------------------------------------------------------------------------------------------
        			| `mH` off	| `mH` on, no gaps	| `mH` on, with gaps	| gaps situation
--------------------------------------------------------------------------------------------------------
no eager-loading	|   51		|	56				|	59 					| all gaps filled in
--------------------------------------------------------------------------------------------------------
.with()				|	47		|	47				|	47 					| no gaps filled in
--------------------------------------------------------------------------------------------------------
.eagerly() 			|	49		|	54				|	57 					| 1 out of 3 that needed it
--------------------------------------------------------------------------------------------------------
.eagerly('myAlias')	|	47		|	52				|	55 					| 1 out of 3 that needed it
--------------------------------------------------------------------------------------------------------

Results with this PR:

--------------------------------------------------------------------------------------------------------
        			| `mH` off	| `mH` on, no gaps	| `mH` on, with gaps	| gaps situation
--------------------------------------------------------------------------------------------------------
no eager-loading	|   51		|	56				|	59 					| all gaps filled in
--------------------------------------------------------------------------------------------------------
.with()				|	47		|	48				|	51 					| all gaps filled in
--------------------------------------------------------------------------------------------------------
.eagerly() 			|	49		|	52				|	61 					| all gaps filled in
--------------------------------------------------------------------------------------------------------
.eagerly('myAlias')	|	47		|	48				|	51 					| all gaps filled in
--------------------------------------------------------------------------------------------------------
  • with maintainHierarchy off, the count and results are the same
  • with maintainHierarchy on, but no gaps to fill, the .with() and .eagerly('myAlias') require the same number of queries to achieve the same results; .eagerly() requires more queries than before, and the higher count compared to .with() and .eagerly('myAlias') comes from multiple field layout providers
  • with maintainHierarchy on, and gaps to fill, the .with() and .eagerly('myAlias') require the same number of queries to achieve the same results; .eagerly() requires more queries than before, and the higher count compared to .with() and .eagerly('myAlias') comes from multiple field layout providers and the fact that it “re-triggers its full eager-loading computation once per distinct field-layout provider that shares the field”

More info:

  • first commit defers filling the gaps (when normalising Categories field value) until ElementQuery::EVENT_BEFORE_PREPARE (similar to what BaseRelationField does with multi-instance relation fields and joining in the relations table values)
  • the second commit (written mostly by Claude) ensures the gaps are filled, and the branch limit is applied when eager loading the Categories field with maintainHierarchy on

New recommendation:

  • if you’re eager-loading a Categories field (especially with maintainHierarchy turned on) used across multiple field layout providers (e.g. multiple entry types), it’s advisable to pass an alias to the .eagerly() method to optimise the number of queries needed to fulfil the task

If anyone has a better idea on how to fix this, LMK.

An argument could also be made that since we’re not filling in the gaps for other Relation fields that support maintaining hierarchy (e.g. Entries) and since .with() was not filling in the gaps before, then .eagerly() shouldn’t do it either. If that’s what we want to go with, LMK and I can adjust.

Related issues

n/a; original discrepancy in the number of queries came from @tommysvr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant