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

[DDC-2224] Honor convertToDatabaseValueSQL() in DQL query parameters #1339

Merged
merged 2 commits into from Mar 25, 2015

Conversation

BenMorel
Copy link
Contributor

This is a follow-up to the abandoned #574 by @mnapoli, that fixes DDC-2224.

At the time, @beberlei closed the PR for the following reason, deemed unfixable:

Passing a type into the parameters is not recognized during caching, that means, using a DQL cache, the same DQL statement with (different parameter types) will lead to the same SQL being generated, depending on if the first or the second set parameter query is executed first.

This PR re-integrates the original fix, and offers a solution to the above issue: take the parameter types into account when checking the local ParserResult and the query cache.

In addition to the test for the DDC-2224 issue, I added a test to ensure that changing a parameter type invalidates the cache.

@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-3625

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

@@ -250,7 +265,7 @@ private function _parse()
return $this->_parserResult;
}

$hash = $this->_getQueryCacheId();
$hash = $this->_getQueryCacheId($types);
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with the rest of the class, I'd say that _getQueryCacheId directly accesses $this->_parsedTypes instead, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll change this!

@Ocramius Ocramius removed the Delayed label Mar 23, 2015
@Ocramius Ocramius self-assigned this Mar 25, 2015
Ocramius added a commit that referenced this pull request Mar 25, 2015
[DDC-2224] Honor convertToDatabaseValueSQL() in DQL query parameters
@Ocramius Ocramius merged commit cf1ecff into doctrine:master Mar 25, 2015
@Ocramius
Copy link
Member

👍

1 similar comment
@mnapoli
Copy link
Contributor

mnapoli commented Mar 25, 2015

👍

@BenMorel
Copy link
Contributor Author

Great, thanks for merging! It would be nice to change DDC-2224 from "invalid" to "fixed" also.

Any chance this fix can land in the 2.5.0 release?

@BenMorel BenMorel deleted the dqlcustomtype branch March 25, 2015 12:04
@Ocramius
Copy link
Member

@BenMorel this will land in 2.5.0

@BenMorel
Copy link
Contributor Author

@Ocramius 👍 Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants