Coming from #999530-14: Upgrade Media Gallery for Media's new File Entity (EntityFieldQueryException: Field storage engine not found)

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.

CommentFileSizeAuthor
#1 efq_invalid_field-1038030-1.patch1.29 KByched
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Status: Active » Needs review
FileSize
1.29 KB

Attached patch detects the error earlier on.

bojanz’s picture

#1: efq_invalid_field-1038030-1.patch queued for re-testing.

bojanz’s picture

Status: Needs review » Reviewed & tested by the community

Patch 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.

yched’s picture

Version: 7.x-dev » 8.x-dev

Thks bojanz. Bumping to 8.x first.

catch’s picture

Looks good to me too.

jackbravo’s picture

This 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?

bojanz’s picture

@jackbravo
It will land in 7.x after it lands in 8.x
Standard procedure.

yched’s picture

Issue tags: +Needs backport to D7

Missing the appropriate tag, though

sun’s picture

Definitely 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.

yched’s picture

@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 :-)

webchick’s picture

Issue tags: +String freeze

Hm. This is going to break string freeze in D7, but I think the consequence is worth it.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +String freeze, +Needs backport to D7

The last submitted patch, efq_invalid_field-1038030-1.patch, failed testing.

webchick’s picture

Status: Needs work » Fixed

Sorry, I see what happened. This is a patch from CVS, so needs patch -p0.

Committed to 8.x and 7.x.

Status: Fixed » Closed (fixed)
Issue tags: -String freeze, -Needs backport to D7

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