While migrating the EFQ code in #1188388: Entity translation UI in core to EFQ2 I stumbled upon this: I needed to count the number of field values for a certain field, which may be shared among different entities (this is the scenario I was using for testing). The count query always returned the total number of values no matter which entity type the entity query was concerning: if I had 3 values, one for nodes, one for comments and one for taxonomy the count query returned 3 for all the entity types instead of one.

CommentFileSizeAuthor
#15 1828790_15.patch3.48 KBchx
#14 1828790_14.patch3.56 KBchx
#14 1828790_14_test_only.patch1.57 KBchx
#13 1828790-test_entity_count_query_wip.do_not_test.patch1.6 KBpfrenssen
#7 eq-count_shared_fields-1828790-7.patch1.99 KBplach
#1 eq-count_shared_fields-1828790-1.patch1.84 KBplach
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Title: Entity query count not working with fields shared among different entities » Entity count query not working with fields shared among different entities
Status: Active » Needs review
FileSize
eq-count_shared_fields-1828790-1.patch1.84 KB

It seems we are missing an entity type condition in the SQL query.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Title: Entity count query not working with fields shared among different entities » Entity count query not working with fields shared among different entity types

Better title

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Great find.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs work » Needs review
Issue tags: -Needs tests

#1: eq-count_shared_fields-1828790-1.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests

The last submitted patch, eq-count_shared_fields-1828790-1.patch, failed testing.

chx’s picture

chx CreditAttribution: chx commented

You need to add to the join condition otherwise LEFT JOINs break.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
FileSize
eq-count_shared_fields-1828790-7.patch1.99 KB

This should be ok, still needs tests though.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs work » Needs review
chx’s picture

chx CreditAttribution: chx commented

pfrenssen is working on a test (and I am helping).

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

FWIW #9 actually fixed the OP case.

Status: Needs review » Needs work
Issue tags: -Needs tests

The last submitted patch, eq-count_shared_fields-1828790-7.patch, failed testing.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs work » Needs review
Issue tags: +Needs tests

#7: eq-count_shared_fields-1828790-7.patch queued for re-testing.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
FileSize
1828790-test_entity_count_query_wip.do_not_test.patch1.6 KB

Here's what I have so far. This still needs work but the office I'm working at is closing. I'm continuing tomorrow.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community
FileSize
1828790_14_test_only.patch1.57 KB
1828790_14.patch3.56 KB

As I did little more than adding an assert and comments #13 and added #7 I don't feel I am RTBC'ing my own.

chx’s picture

chx CreditAttribution: chx commented
FileSize
1828790_15.patch3.48 KB

Even better comments after talking to plach.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Status: Fixed » Needs review
Issue tags: -Needs tests

#15: 1828790_15.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests

The last submitted patch, 1828790_15.patch, failed testing.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs work » Fixed

This has already been committed, it is not necessary to test the patch any more.

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