Problem/Motivation

When field names a too long, the actual 'real table' used for field storage is a hashed snippet of the actual field name. E.g. field_collection_item_5c4820gfrd (choosing this as it's a good example of an entity with a long name, which amplifies the likelihood of the problem).

Trying to create a relationship on this field, for example an entity reference field, will result in a lot of 'table not found' sql errors.

Proposed resolution

After some digging it ended up being a problem when HandlerBase::getTableJoin() tries to create a join to add to the query for the relationship. The table join data that would be used is not defined for fields, so it just relies on the views table name (which is the regular field name (not shortened)). If we add join table items for the actual table, views will use this in the joins and peace is restored.

Remaining tasks

Tests

User interface changes

N/a

API changes

Table key added to join table data for field views data.

Data model changes

N/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip created an issue. See original summary.

damiankloip’s picture

Status: Active » Needs review
FileSize
1.95 KB

Here is the fix. Tests on their way but want to see if anything is breaking with this. Don't think it should but you just don't know with Drupal.

Status: Needs review » Needs work

The last submitted patch, 2: 2755043.patch, failed testing.

damiankloip’s picture

With a failing test. I had to extend the tests to use the entity_test_mul_changed entity type as that gives us a name long enough that it will get truncated and hashed for field storage.

The last submitted patch, 4: 2755043-4-tests-only-FAIL.patch, failed testing.

catch’s picture

+++ b/core/modules/field/tests/src/Kernel/EntityReference/Views/EntityReferenceRelationshipTest.php
@@ -62,6 +65,12 @@ protected function setUp($import_test_views = TRUE) {
+    // tables created will be grater than 48 chars long.

greater

But that's fixable on commit.

Rest looks fine.

dawehner’s picture

The patch itself looks good, but I'm actively wondering whether we also need to fix EntityViewsData as well. I guess though as we use the table mapping automatically we are fine?

damiankloip’s picture

Yes. I couldn't find an issue with the stuff mapped from EntityViewsData, just usage with our default field implementation.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

WFM

  • catch committed 3174709 on 8.3.x
    Issue #2755043 by damiankloip: VIews field relationships break when the...

  • catch committed d9ae35c on 8.2.x
    Issue #2755043 by damiankloip: VIews field relationships break when the...
catch’s picture

Version: 8.2.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to all three 8.x branches, thank!

  • catch committed 74cd4ce on 8.1.x
    Issue #2755043 by damiankloip: VIews field relationships break when the...
jibran’s picture

I have to fix that for DER as well then.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.