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

Comments

mcdruid created an issue. See original summary.

mcdruid’s picture

Issue summary: View changes
mcdruid’s picture

Status: Active » Needs review
StatusFileSize
new1 KB

The schema info for the entity is already available, so perhaps something simple like this would work..

mcdruid’s picture

Issue tags: +PostgreSQL
StatusFileSize
new1.7 KB
new1.82 KB
new684 bytes

The comments mention that $condition can 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.:

Exception Uncaught e database.inc       262 PDO->prepare()                     
    PDOException: SQLSTATE[HY000]: General error: 1 near "ESCAPE":
    syntax error: SELECT taxonomy_term_data.tid AS entity_id, :entity_type AS
    entity_type, NULL AS revision_id
    FROM 
    {taxonomy_term_data} taxonomy_term_data
    WHERE  (taxonomy_term_data.name LIKE :db_condition_placeholder_0_0,
    :db_condition_placeholder_0_1 ESCAPE '\') ; Array
    (
        [:entity_type] => taxonomy_term
        [:db_condition_placeholder_0_0] => hRzsFtm1
        [:db_condition_placeholder_0_1] => wOz545cY
    )
     in EntityCacheControllerHelper::entityCacheLoad() (line 112 of
    /var/www/html/sites/all/modules/contrib/entitycache/includes/entitycache.entitycachecontrollerhelper.inc).

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.

mcdruid’s picture

StatusFileSize
new816 bytes
new1.82 KB

Oops - I accidentally removed an isset().

This is looking good to me other than that though.

  • mcdruid committed d1053449 on 7.x-1.x
    Issue #3376606 by mcdruid: EntityCacheControllerHelper::entityCacheLoad...
mcdruid’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

mcdruid’s picture

Correction: it's actually only \TaxonomyTermController::buildQuery that changes the operator to LIKE when 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...