Currently, the fieldOrderBy() method in an EntityFieldQuery will exclude any Entities where the field value is empty.

This was originally discussed as a bug report in #1611438: fieldOrderBy filters out results with empty field values.

It was determined that the exclusion of entities with empty fields was by design for performance reasons.

This behavior should be documented.

Files: 
CommentFileSizeAuthor
#16 1662950_d7_port_16.patch905 bytesbxtaylor
PASSED: [[SimpleTest]]: [MySQL] 39,240 pass(es).
[ View ]
#11 1662950_11.patch1.05 KBchx
PASSED: [[SimpleTest]]: [MySQL] 37,046 pass(es).
[ View ]
#10 document-that-EntityField-1662950-10.patch1.18 KBmjonesdinero
PASSED: [[SimpleTest]]: [MySQL] 37,040 pass(es).
[ View ]
#7 document-that-EntityField-1662950-7.patch740 bytesmjonesdinero
PASSED: [[SimpleTest]]: [MySQL] 37,036 pass(es).
[ View ]
#2 document_that_EntityField-1662950-2.patch781 bytesmjonesdinero
PASSED: [[SimpleTest]]: [MySQL] 37,005 pass(es).
[ View ]

Comments

jhodgdon’s picture

http://api.drupal.org/api/drupal/core!modules!entity!lib!Drupal!entity!E...

Looks like a good Novice project to add a line to the documentation with this note.

mjonesdinero’s picture

Assigned:Unassigned» mjonesdinero
Status:Active» Needs review
StatusFileSize
new781 bytes
PASSED: [[SimpleTest]]: [MySQL] 37,005 pass(es).
[ View ]

Attached is the patch please feedback if there is some changes

jhodgdon’s picture

Status:Needs review» Needs work

Thanks for the patch! The grammar needs a little adjustment though -- the first sentence isn't a sentence. Actually, I think it should probably start with "Note that...". I would also leave out the "this is by design" sentence. Just state what the function does.

bxtaylor’s picture

Agree with @jhodgdon, should read something like: "Note that Entities with empty field values will be excluded from the EntityFieldQuery results when using the fieldOrderBy() method."

jhodgdon’s picture

I agree with #4, except that the word "entitites" does not need to be capitalized.

chx’s picture

You can add the same to fieldCondition too. Thanks!

mjonesdinero’s picture

Status:Needs work» Needs review
StatusFileSize
new740 bytes
PASSED: [[SimpleTest]]: [MySQL] 37,036 pass(es).
[ View ]

update the what bxtaylor and jhodgdon decided

jhodgdon’s picture

Thanks!

A few things to fix:
a) The existing paragraph about "If called multiple times" should not be removed. This new text should probably be added as a separate paragraph.

b) See #6.

jhodgdon’s picture

Status:Needs review» Needs work

forgot status

mjonesdinero’s picture

Status:Needs work» Needs review
StatusFileSize
new1.18 KB
PASSED: [[SimpleTest]]: [MySQL] 37,040 pass(es).
[ View ]

thanks jhodgdon

update patch now

chx’s picture

StatusFileSize
new1.05 KB
PASSED: [[SimpleTest]]: [MySQL] 37,046 pass(es).
[ View ]

I moved the second to the fieldOrderBy method itself, not sure why it was on entityOrderBy? And changed the method names to simply "this method".

bxtaylor’s picture

Status:Needs review» Reviewed & tested by the community

Patch in #11 looks good to me. Thanks, chx!

mjonesdinero’s picture

Status:Reviewed & tested by the community» Needs review

thanks for correcting chx

mjonesdinero’s picture

Status:Needs review» Reviewed & tested by the community
jhodgdon’s picture

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

Thanks! Committed to 8.x. The class is in a different file in 7.x, so this needs a port.

bxtaylor’s picture

StatusFileSize
new905 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,240 pass(es).
[ View ]

Here's the patch for D7.

bxtaylor’s picture

Status:Patch (to be ported)» Needs review
jhodgdon’s picture

This patch doesn't seem to include as much as the 8.x patch. Is that because one of the methods doesn't exist in 7.x, or because the patch is incorrect?

bxtaylor’s picture

I'm looking at the two patches side-by-side:
http://drupal.org/files/1662950_11.patch (chx)
http://drupal.org/files/1662950_d7_port_16.patch (mine)

and there only seems to be a one line difference.

Seem the same, otherwise.

chx’s picture

Status:Needs review» Reviewed & tested by the community

I copied the hunks (no headers) into two files and ran diff over them. No textual difference.

jhodgdon’s picture

My fault, sorry! I'll get that patch committed. Thanks!

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 7.x. Thanks again everyone!

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