Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
This is basically a follow-up of #2391217: Support base fields with multiple columns in entity queries.
When querying a multi-property field the fallback to the main property does not work if the field is a base field. So while this query works fine:
\Drupal::entityTypeManager()->getStorage('node')->getQuery()
->condition('body', 'The value of body.value (there is also body.format)')
->execute();
The following fails:
\Drupal::entityTypeManager()->getStorage('taxonomy_term')->getQuery()
->condition('description', 'The value of description.value (there is also description.format)')
->execute();
In particular the following exception is thrown:
Drupal\Core\Entity\Query\QueryException: 'description' not found
The only difference is that body is a configurable field on nodes and description is a base field for terms.
Steps to reproduce
Fix the error!
Proposed resolution
Remaining tasks
User interface changes
None.
API changes
None.
Data model changes
None.
Release notes snippet
None.
Issue fork drupal-3189174
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
tstoecklerAdded a test exposing the bug. Had set the version 8.9.x as I thought that was the process for bugs, but that meant the issue branch was based on 8.9.x and not 9.2.x so fixing that now. Not sure if that is correct, though.
Comment #4
tstoecklerSo the first, test-only commit nicely shows the bug.
Provided a fix now in the MR. On the one hand the added code feels like adding another band-aid on top of a whole mess of band-aids, which is
\Drupal\Core\Entity\Query\Sql\Tables, on the other hand this fits reasonably well into the existing code, so - while I guess it doesn't make that whole method more understandable - it doesn't really make it less so (or only marginably less so). So I personally think this is the best solution in order to fix the bug while a proper refactor of that class can be done in #2423323: Streamline EntityQuery's Tables::addField().Comment #5
tstoecklerOK, this one should be green.
I guess I should have run all of
EntityQueryTestbefore pushing the fix before, so sorry for the needless test run, although in my defense the fact that$key < $count(apparently! (still don't grok why...)) implies that$field_storageis set but it may not be set otherwise is incredibly obscure even byTables::addField()standards...Comment #6
daffie CreditAttribution: daffie commentedWhen I ran the patch with only the added test, then the EntityQueryTests fails.
When I ran the complete patch against a PostgreSQL database, then the EntityQueryTests passes.
The MR provides a fix and testing for the bug as described by the IS.
All code changes look good to me.
For me it is RTBC.
@tstoeckler: Good work!
Comment #7
daffie CreditAttribution: daffie commentedComment #8
catchFix looks fine but found an issue in one comment.
Comment #9
tstoecklerFixed the comment issue, thanks!
Comment #10
daffie CreditAttribution: daffie commentedThe comment has been fixed.
Back to RTBC.
Comment #13
catchCommitted/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!