Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed query cache id generation: added platform to hash #1075

Merged
merged 2 commits into from
Jun 27, 2014

Conversation

vilartoni
Copy link
Contributor

There's an issue with the query cache id generation in Doctrine\ORM\Query::_getQueryCacheId().

If you happen to use different connections to different platforms on the same project and you're using the query cache, you will get an exception the moment you try to execute a query which SQL is different depending on the platform and it has been previously cached for the other platform, as they will share the same cache id.

In order to reproduce the bug it is sufficient with using the Doctrine Paginator in a query:

$query = $queryBuilder->setFirstResult(0)
    ->setMaxResults(50)
    ->getQuery()
    ->getResult();

If we run the query for the first time with an empty cache in an Oracle connection and later on we try to run the same query in a MySQL connection, we get the following exception:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'ROWNUM' in 'where clause'

As it's trying to execute the SQL for Oracle in the MySQL connection due to the same cache id.

This issue can be easily fixed just by taking the platform type into account in the cache id generation.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3198

We use Jira to track the state of pull requests and the versions they got
included in.

@Ocramius
Copy link
Member

@vilartoni are you using the same cache across different connections on purpose?

@vilartoni
Copy link
Contributor Author

@Ocramius Yes. We're using a common cache server for all cached queries.

@Ocramius
Copy link
Member

Can you check if the travis failure here is related to your change?

@vilartoni
Copy link
Contributor Author

Sure. I'd say it is not. It's something to do with the MOD() dql function failing in postgresql.

@Ocramius
Copy link
Member

restarted that worker

@vilartoni
Copy link
Contributor Author

All green.

@Ocramius Ocramius self-assigned this Jun 27, 2014
return md5(
$this->getDql() . serialize($this->_hints) .
$this->getDql() . serialize($this->_hints) . $platform .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add it as a new line and use &platform=? Looking good otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants