Contrib.social

Entity field relationship queries for multi column field items stored in shared tables are broken

Open on Drupal.org →
Created on 30 April 2022, over 2 years ago
Updated 5 July 2024, 5 months ago

Problem/Motivation

The rather convoluted code in Tables.php when a specifier is not the last and also a shared table is used then the column mapping is not called and the specifier will be used as the SQL column name which bombs.

Steps to reproduce

  1. Create a custom entity with a file type base field
  2. Create an entity query with fieldname.entity.filename condition.
  3. Run the query
  4. Read the error message 😢

namespace Drupal\repro\Entity;                                                                                                                         
                                                                                                                                                       
use Drupal\Core\Entity\ContentEntityBase;                                                                                                              
use Drupal\Core\Entity\ContentEntityInterface;                                                                                                         
use Drupal\Core\Entity\EntityTypeInterface;                                                                                                            
use Drupal\Core\Field\BaseFieldDefinition;                                                                                                             
                                                                                                                                                       
/**                                                                                                                                                    
 * Bug repro.                                                                                                                                          
 *                                                                                                                                                     
 * @ContentEntityType(                                                                                                                                 
 *   id = "repro",                                                                                                                                     
 *   base_table = "repro",                                                                                                                             
 *   entity_keys = {                                                                                                                                   
 *     "id" = "id",                                                                                                                                    
 *   }                                                                                                                                                 
 * )                                                                                                                                                   
 */                                                                                                                                                    
class Repro extends ContentEntityBase implements ContentEntityInterface {                                                                              
  public static function baseFieldDefinitions(EntityTypeInterface $entity_type): array {                                                               
    $fields = parent::baseFieldDefinitions($entity_type);                                                                                              
    $fields['file'] = BaseFieldDefinition::create('file');                                                                                             
    return $fields;                                                                                                                                    
  }                                                                                                                                                    
  public static function test(): array {                                                                                                               
    $storage = \Drupal::entityTypeManager()->getStorage('repro');                                                                                      
    $query = $storage->getQuery()->accessCheck(FALSE);                                                                                                 
    $query->condition('file.entity.filename', 'whatever.doc');                                                                                         
    return $query->execute();                                                                                                                          
  }                                                                                                                                                    
}                                                                                                                                                      


$ drush eval 'Drupal\repro\Entity\Repro::test()'

In Tables.php line 369:
                    
  'file' not found

Proposed resolution

Fix the condition logic in Tables.

Make sure the change doesn't break things.

User interface changes

None

API changes

The documented behavior of the Entity Query API will work correctly.

Data model changes

Don't know the internals well enough to answer this one.

Release notes snippet

The standard "bug was fixed" note?

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Entity  →

Last updated about 14 hours ago

Created by

🇺🇸United States bkline Virginia

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇩🇪Germany geek-merlin Freiburg, Germany

    Is this stalled? It blocks the other issue.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    30,377 pass, 2 fail
  • Comment 10 months ago →
    🇩🇪Germany geek-merlin Freiburg, Germany

    Unblocking this as the other issue is stalled, and imho no blocker for working on this.

    > and I don't actually understand what this example syntax would query for:
    > tid.referenced_by:tags:node.nid

    "Referenced by field tags of a node" ;-)

  • Comment 5 months ago →
    🇬🇧United Kingdom joachim

    Made an issue fork, and committed the patches to it, then rebased on 11.x.

    Checked that the diff to 11.x is the same as patch file 3278083-51.patch.

  • Status changed to Needs review 5 months ago
  • Comment 5 months ago →
    🇬🇧United Kingdom joachim

    Made a number of tweaks -- mostly docs & formatting fixes.

    Uncommented the deprecation.

    Also, addressed this

    > I'm a bit spooked by this class property which gets overwritten each time a new field is added to the query.

    by removing the $this->joinType and using a passed parameter instead.

  • Merge request !8634#3278083 → (Open) created by joachim
  • Pipeline finished with Failed
    5 months ago
    Total: 213s
    #214058
  • Comment 5 months ago →
    🇬🇧United Kingdom joachim

    I'm still iffy about the EntityQueryRelationshipMultiplePropertyBaseFieldTest test class being empty, but I don't think there's a better way to do it.

    I tried a PHPUnit data provider with the code from createField() in a callable... but PHPUnit didn't like that at all!!! :D

  • Pipeline finished with Failed
    5 months ago
    Total: 190s
    #214084
  • Pipeline finished with Failed
    5 months ago
    Total: 474s
    #214615
  • Status changed to Needs work 5 months ago
  • Comment 5 months ago →
    🇺🇸United States smustgrave

    Can the issue summary be updated to include the fix includes needing createField()

    Also appears to have a repeating failing kernel test.

    Left some comments on the MR.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024