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

Call detectDatabasePlatform only once #781

Closed
wants to merge 1 commit into from
Closed

Call detectDatabasePlatform only once #781

wants to merge 1 commit into from

Conversation

rosier
Copy link
Contributor

@rosier rosier commented Jan 22, 2015

Database platform detection is triggered twice if Doctrine/DBAL/Connection::getDatabasePlatform() is called before Doctrine/DBAL/Connection::connect()

@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/DBAL-1127

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

@Ocramius
Copy link
Member

@rosier requires a test case

@@ -1597,7 +1593,7 @@ public function ping()
}

try {
$this->query($this->platform->getDummySelectSQL());
$this->query($this->getDatabasePlatform()->getDummySelectSQL());
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change necessary? Connection::ping() calls Connection::connect() as the beginning, so it ensures the platform is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The platform isn't set anymore in connect() with this PR.

In the current version of Connection the following calls are triggered:
getDatabasePlatform() => detectDatabasePlatform() => getDatabasePlatformVersion() => connect() at this point because the platform isn't set yet connect() calls detectDatabasePlatform() again.

This PR removes the call to detectDatabasePlatform() from connect() to stop that from happening.

@rosier
Copy link
Contributor Author

rosier commented Jan 23, 2015

@Ocramius Connection::detectDatabasePlatform() is a private function. I'm not sure if there is a good way to test that it is called only once.

@Ocramius
Copy link
Member

@rosier try looking at the wrapped driver: that one can be mocked

deeky666 added a commit that referenced this pull request Jan 10, 2016
Call detectDatabasePlatform only once

fixes #1068
@deeky666
Copy link
Member

Merged manually via de75561. Verified the issue by adding a test. Thanks @rosier !

@deeky666 deeky666 closed this Jan 10, 2016
@deeky666 deeky666 added this to the 2.6 milestone Jan 10, 2016
@deeky666 deeky666 self-assigned this Jan 10, 2016
@Ocramius Ocramius changed the title Call detectDatabasePlatform only once Call detectDatabasePlatform only once Jul 22, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants