EntityFieldQuery currently returns an incorrect entity count when you query a field that has multiple translations.

If you create a node with a translatable body field, and add 3 translations, querying for the entity like this:

<?php
$query
= new EntityFieldQuery();
$entities = $query->fieldCondition('body')->execute();
?>

will return one entity, but if you do the same thing with a count
<?php
$query
= new EntityFieldQuery();
$count = $query->fieldCondition('body')->count()->execute();
?>

the result is three.

This also means that $query->ordered_results will have duplicates.

Files: 
CommentFileSizeAuthor
#16 efq-translatable-field-count-1292922-16-test.patch1.72 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] 36,473 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#16 efq-translatable-field-count-1292922-16.patch2.42 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 36,481 pass(es).
[ View ]
#8 efq-translatable-field-count-1292922-8.patch2.66 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 33,260 pass(es).
[ View ]
#8 efq-translatable-field-count-1292922-8-test.patch1.96 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] 33,275 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#6 efq-translatable-field-count-1292922-6-test.patch1.96 KBloganfsmyth
FAILED: [[SimpleTest]]: [MySQL] 33,254 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#6 efq-translatable-field-count-1292922-6.patch2.66 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 33,253 pass(es).
[ View ]
#4 efq-translatable-field-count-1292922-4-test.patch1.95 KBplach
FAILED: [[SimpleTest]]: [MySQL] 33,258 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#3 efq-translatable-field-count-1292922-3.patch2.65 KBloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 33,262 pass(es).
[ View ]
#1 efq-translatable-field-count-1292922-1.patch713 bytesloganfsmyth
PASSED: [[SimpleTest]]: [MySQL] 33,114 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new713 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,114 pass(es).
[ View ]

This was enough to get it working for me.

I don't know if using $field['translatable'] directly is acceptable, or if we need to use something like this, http://api.drupal.org/api/drupal/modules--field--field.multilingual.inc/..., but then you need to have entity_type nailed down.

Status:Needs review» Needs work

From a quick review the fix looks correct, I think we need some tests covering the bug.

Status:Needs work» Needs review
StatusFileSize
new2.65 KB
PASSED: [[SimpleTest]]: [MySQL] 33,262 pass(es).
[ View ]

Added a basic test that creates a single entity with a translatable field in two languages, with cardinality one.

This patch also includes adding entity_query.test to entity.info's file list, because it seems to have been missed when entities moved the a module. I've seen that particular fix one of the other entity related patches too, but it hasn't been committed yet.

Issue tags:+needs backport to D7
StatusFileSize
new1.95 KB
FAILED: [[SimpleTest]]: [MySQL] 33,258 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Stupid coding standard things to be fixed below. Attaching a test-only patch which is supposed to fail to prove that the tests capture the bug. Also, this will need to be backported.

+++ b/modules/entity/tests/entity_query.test
@@ -1048,6 +1048,42 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
+    // Make a test field translatable AND cardinality 1

Missing trailing dot.

+++ b/modules/entity/tests/entity_query.test
@@ -1048,6 +1048,42 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
+    // Set fields in two languages with 1 field value

Missing trailing dot.

+++ b/modules/entity/tests/entity_query.test
@@ -1048,6 +1048,42 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
+    // Look up number of results when querying a single entity with multilingual field values.

Comment does not wrap at column 80.

-1 days to next Drupal core point release.

Status:Needs review» Needs work

The last submitted patch, efq-translatable-field-count-1292922-4-test.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.66 KB
PASSED: [[SimpleTest]]: [MySQL] 33,253 pass(es).
[ View ]
new1.96 KB
FAILED: [[SimpleTest]]: [MySQL] 33,254 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Here's an update for coding standards, and a test-only patch to match.

Status:Needs review» Needs work

Grrreat, but...

+++ b/modules/entity/tests/entity_query.test
@@ -1048,6 +1048,43 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
+    // Look up number of results when querying a single entity
+    // with multilingual field values.

...comments should wrap as near as possible to column 80. In this case:

    // Look up number of results when querying a single entity with multilingual
    // field values.

See http://drupal.org/coding-standards for details.

Status:Needs work» Needs review
StatusFileSize
new1.96 KB
FAILED: [[SimpleTest]]: [MySQL] 33,275 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
new2.66 KB
PASSED: [[SimpleTest]]: [MySQL] 33,260 pass(es).
[ View ]

Ah, didn't realize that, thanks.

Is DISTINCT what you want? Shouldn't this be a documentation patch to urge you to specify a language?

Status:Needs review» Needs work

The last submitted patch, efq-translatable-field-count-1292922-8-test.patch, failed testing.

Status:Needs work» Needs review

I think in this case DISTINCT makes the most sense, as we may not know or want to query by the language. Also, the distinct is already there in any case whenever the cardinality isn't '1', so I felt this change wouldn't be a drastic addition. It doesn't really make sense that you could get one result entity when you run the query itself, but then get a count of more than one when you do a count query. Even if it were documented, it would be a fairly strange behavior to have.

As an example of not knowing the language to query by, I noticed this issue while working on Entity Translation because we want to look up all entities with values for a certain field, and then only later look up field values based on that particular entity's language.

I also think that distinct is the way to go here: since an entity has only one language there is no point in explictly specifying one to obtain the correct entity count. Unless @chx disagrees with this reasoning, I'd say #8 is RTBC.

Status:Needs review» Reviewed & tested by the community

It is.

Issue tags:+translatable fields

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

That testX() function needs some PHPDoc. I added:

  /**
   * Tests querying translatable fields.
   */

And committed/pushed to 8.x. Thanks!

Needs a backport for D7.

Status:Patch (to be ported)» Needs review
StatusFileSize
new2.42 KB
PASSED: [[SimpleTest]]: [MySQL] 36,481 pass(es).
[ View ]
new1.72 KB
FAILED: [[SimpleTest]]: [MySQL] 36,473 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Backported to D7 and added webchick's PHPDoc.

Status:Needs review» Reviewed & tested by the community

nice!

Status:Reviewed & tested by the community» Fixed

Thank you!

Committed and pushed to 7.x.

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

Issue summary:View changes

Updated issue summary.