Ticket #1427 (closed defect: fixed)

Opened 10 months ago

Last modified 10 months ago

Query cache has collisions between queries of different types

Reported by: chorizo Owned by: jwage
Priority: critical Milestone: 1.0.1
Component: Attributes Version: 1.0.0
Severity: Keywords:
Cc: Has Test: yes
Status: Pending Core Response Has Patch: yes

Description

The second of these two queries always breaks because Doctrine_Query::getDql() returns the same string for both of them, which gets hashed as the key to the query:

// DELETE query
Doctrine_Query::create()
->delete()
->from('SpaceMemberRequest')
->where('user_id = ? AND namespace_id = ?', array($user->id, $this->id))
->execute();   
// SELECT query
Doctrine::getTable('SpaceMemberRequest')->createQuery()
->where('user_id = ? AND namespace_id = ?', array($userId, $spaceId))
->fetchOne();      

I believe this might be an issue for the result cache as well, since it uses getDql along with the parameters as the Hash.

Here's a test:

    public function testQueryCacheCollisionOnType()
    {
        $cache = new Doctrine_Cache_Array();

        // DELETE
        $q = Doctrine_Query::create()
          ->delete()
          ->from('User')
          ->where('name = ?', array('walhala'))
          ->useQueryCache($cache);
        $coll = $q->execute();
        $this->assertEqual($cache->count(), 1);

        // SELECT
        $q = Doctrine_Query::create()
          ->from('User')
          ->where('name = ?', array('walhala'))
          ->useQueryCache($cache);
        $coll = $q->execute();
        $this->assertEqual($cache->count(), 2);

        // DELETE again
        $q = Doctrine_Query::create()
          ->delete()
          ->from('User')
          ->where('name = ?', array('walhala'))
          ->useQueryCache($cache);
        $coll = $q->execute();
        $this->assertEqual($cache->count(), 2);

        // UPDATE
        $q = Doctrine_Query::create()
          ->update('User')
          ->from('User')
          ->set('name', '?', array('walhala2'))
          ->where('name = ?', array('walhala'))
          ->useQueryCache($cache);
        $coll = $q->execute();
        $this->assertEqual($cache->count(), 3);
    }

Here's my patch for Doctrine_Query_Abstract::getDql, which changes it to return valid DQL, and fixes the cache collision above:

    public function getDql()
    {
        $q = '';
        if ($this->_type == Doctrine_Query::SELECT) {
          $q .= ( ! empty($this->_dqlParts['select']))?  'SELECT '    . implode(', ', $this->_dqlParts['select']) : '';
          $q .= ( ! empty($this->_dqlParts['from']))?    ' FROM '     . implode(' ', $this->_dqlParts['from']) : '';
        } else if ($this->_type == Doctrine_Query::DELETE) {
          $q .= 'DELETE';
          $q .= ( ! empty($this->_dqlParts['from']))?    ' FROM '     . implode(' ', $this->_dqlParts['from']) : '';
        } else if ($this->_type == Doctrine_Query::UPDATE) {
          $q .= 'UPDATE ';
          $q .= ( ! empty($this->_dqlParts['from']))? implode(' ', $this->_dqlParts['from']) : '';
          $q .= ( ! empty($this->_dqlParts['set']))? ' SET ' . implode(' ', $this->_dqlParts['set']) : '';
        }
        $q .= ( ! empty($this->_dqlParts['where']))?   ' WHERE '    . implode(' ', $this->_dqlParts['where']) : '';
        $q .= ( ! empty($this->_dqlParts['groupby']))? ' GROUP BY ' . implode(', ', $this->_dqlParts['groupby']) : '';
        $q .= ( ! empty($this->_dqlParts['having']))?  ' HAVING '   . implode(' AND ', $this->_dqlParts['having']) : '';
        $q .= ( ! empty($this->_dqlParts['orderby']))? ' ORDER BY ' . implode(', ', $this->_dqlParts['orderby']) : '';
        $q .= ( ! empty($this->_dqlParts['limit']))?   ' LIMIT '    . implode(' ', $this->_dqlParts['limit']) : '';
        $q .= ( ! empty($this->_dqlParts['offset']))?  ' OFFSET '   . implode(' ', $this->_dqlParts['offset']) : '';

        return $q;
    }

Change History

Changed 10 months ago by romanb

  • status changed from new to closed
  • resolution set to fixed

(In [4876]) Fixed #1427. Thanks for the patch.

Changed 10 months ago by romanb

  • milestone set to 1.0.1
Note: See TracTickets for help on using tickets.