When calling fieldCondition($field_name) or fieldOrder($field_name) where field_info_field($field) returns NULL (field name doesn't exist), we fail in strange, non tale-telling ways - possibly with a "Field storage engine not found" exception down the road if the field was the only one in the query, or with (I guess, untested) a SQL error of there were other fields.
Comment | File | Size | Author |
---|---|---|---|
#1 | efq_invalid_field-1038030-1.patch | 1.29 KB | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedAttached patch detects the error earlier on.
Comment #2
bojanz CreditAttribution: bojanz commented#1: efq_invalid_field-1038030-1.patch queued for re-testing.
Comment #3
bojanz CreditAttribution: bojanz commentedPatch still applies (although with -p0), tests still pass, approach makes sense.
I was actually about to write this patch myself (wrote EntityFieldQuery support for Per-Bundle-Storage and was thinking how unfriendly the unknown field name business is), and stumbled into the issue by accident.
Comment #4
yched CreditAttribution: yched commentedThks bojanz. Bumping to 8.x first.
Comment #5
catchLooks good to me too.
Comment #6
jackbravo CreditAttribution: jackbravo commentedThis doesn't seem to add any new functionality, and instead fix something that's very confusing in Drupal 7. Is it too dangerous to port this to D7?
Comment #7
bojanz CreditAttribution: bojanz commented@jackbravo
It will land in 7.x after it lands in 8.x
Standard procedure.
Comment #8
yched CreditAttribution: yched commentedMissing the appropriate tag, though
Comment #9
sunDefinitely a step forward. As a follow-up issue for D8, we should come up with a filtered/optimized backtrace debug output for these kind of exceptions, since although this patch adds the info about what field name is unknown, you still have no effin' clue from where in your code base this field was tried to be queried in the first place.
Comment #10
yched CreditAttribution: yched commented@sun re: lack of backtrace info :
true - on that note, see #1067750: Let Field API fail in a tale-telling way on invalid $entity, which desperately needs a bump :-)
Comment #11
webchickHm. This is going to break string freeze in D7, but I think the consequence is worth it.
Comment #12
webchick#1: efq_invalid_field-1038030-1.patch queued for re-testing.
Comment #14
webchickSorry, I see what happened. This is a patch from CVS, so needs patch -p0.
Committed to 8.x and 7.x.