Problem/Motivation
Running entitycache's test suite with sqlite reveals that the following two taxonomy tests fail:
testTaxonomyGetTermByName
fail: [Other] Line 981 of modules/taxonomy/taxonomy.test:
Term loaded with uppercased name.
fail: [Other] Line 985 of modules/taxonomy/taxonomy.test:
Term loaded with lowercased name.
As far as I can see this is because when core runs taxonomy_get_term_by_name() without entitycache, DrupalDefaultEntityController::load does this:
// Build the query.
$query = $this->buildQuery($ids, $conditions, $revision_id);
...which produces a select query with a condition along the lines of base.name LIKE "foo".
Whereas with entitycache enabled, \EntityCacheControllerHelper::entityCacheLoad does this:
if ($conditions) {
$query = new EntityFieldQuery();
$query->entityCondition('entity_type', $controller->entityType);
foreach ($conditions as $property_name => $condition) {
// Note $condition might be multiple values, which are treated as OR
// by default.
$query->propertyCondition($property_name, $condition);
...where EntityFieldQuery::propertyCondition uses a default operator of = for the condition.
With MySQL this typically makes no difference, but with other database systems which are case sensitive by default, there's a difference in behaviour as a result of the different operators / SQL syntax.
Steps to reproduce
Run entitycache tests with sqlite (and probably postgres) e.g.:
---- EntityCacheTaxonomyTermTestCase ----
Status Group Filename Line Function
--------------------------------------------------------------------------------
Fail Other taxonomy.test 981 TaxonomyTermTestCase->testTaxonomyG
Term loaded with uppercased name.
Fail Other taxonomy.test 985 TaxonomyTermTestCase->testTaxonomyG
Term loaded with lowercased name.
Proposed resolution
I suppose to fix this we'd need to do something like explicitly pass an appropriate operator to EntityFieldQuery::propertyCondition for certain conditions.
For example, this quick hack makes the tests pass:
foreach ($conditions as $property_name => $condition) {
// Note $condition might be multiple values, which are treated as OR
// by default.
$operator = NULL;
if ($property_name == 'name') {
$operator = 'LIKE';
}
$query->propertyCondition($property_name, $condition, $operator);
}
However, we'd need to do something like actually look up the property name in the base table's schema to determine whether it's appropriate to set the operator to something other than the default.
Remaining tasks
Outlined above.
User interface changes
n/a
API changes
n/a
Data model changes
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3376606-5.patch | 1.82 KB | mcdruid |
| #5 | interdiff-3376606-4-5.txt | 816 bytes | mcdruid |
| #4 | 3376606-4_noop.patch | 684 bytes | mcdruid |
| #4 | 3376606-4.patch | 1.82 KB | mcdruid |
| #4 | interdiff-3376606-3-4.txt | 1.7 KB | mcdruid |
Comments
Comment #2
mcdruid commentedComment #3
mcdruid commentedThe schema info for the entity is already available, so perhaps something simple like this would work..
Comment #4
mcdruid commentedThe comments mention that
$conditioncan include multiple values, so we should take that into account even if core's existing test coverage doesn't seem to exercise it in this specific case.This patch adds another test that passes multiple term names as the condition. (I'd consider adding this test to core but will leave it here for now).
If we change the operator to LIKE when multiple string conditions are specified, we end up with invalid SQL - e.g.:
I don't think it's worth trying to work around that here; let's just keep it simple and change the operator when there's only one value in a string condition.
Realistically this change isn't going to do much on the majority of real world sites, but having completely clean test coverage for entitycache is worth making this small tweak for if we're going to make one or more new releases.
Including a noop patch so we can verify test failures without the operator change.
Comment #5
mcdruid commentedOops - I accidentally removed an
isset().This is looking good to me other than that though.
Comment #7
mcdruid commentedComment #9
mcdruid commentedCorrection: it's actually only
\TaxonomyTermController::buildQuerythat changes the operator toLIKEwhen specifically adding a condition on the term name in core; entitycache now tries to match this:https://git.drupalcode.org/project/entitycache/-/blob/7.x-1.7/includes/e...