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

BC break between 1.6.7 -> 1.6.8 regarding driver selection #673

Closed
Majkl578 opened this issue Jun 27, 2017 · 17 comments

Comments

Projects
None yet
@Majkl578
Copy link
Contributor

commented Jun 27, 2017

After upgrading from 1.6.7 to 1.6.8, all our tests started failing. While investigating, found that DBAL driver is somehow not picked up correctly.
All functional tests failing with:

Doctrine\DBAL\Exception\DriverException: An exception occured in driver: could not find driver

It suddenly tries to load pdo_mysql instead of pdo_pgsql.

Bisected this breakage to commit 99a62a7.

This is what our DBAL config looks like:

doctrine:
    dbal:
        default_connection: xxx
        connections:
            xxx:
                server_version: 9.4
                driver: "%database_driver%"
                host: "%database_host%"
                port: "%database_port%"
                dbname: "%database_name%"
                user: "%database_user%"
                password: "%database_password%"
                charset: UTF8
                mapping_types:
                    bit: boolean
                    tsvector: string
        types:
            foo:
                class: X\Y\Types\FooType # <-- commented type
@Ocramius

This comment has been minimized.

Copy link
Member

commented Jun 27, 2017

Indeed, $platform = $connection->getDatabasePlatform(); is the culprit here.

@mikeSimonson how much of an issue would a reversal of the bugfix be? I think this one is more critical...

@Ocramius Ocramius added the Bug label Jun 27, 2017

@mikeSimonson

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2017

Does that mean that somehow the $connection->getDatabasePlatform(); call override the server version config value ? Reading the code it seems like it shouldn't be the case (it bails out if it's already set).

@Majkl578 Can you dig further with xdebug to see how that value gets changed ?

@stof

This comment has been minimized.

Copy link
Member

commented Jun 28, 2017

@mikeSimonson $connection->getDatabasePlatform() needs to know the server version, to get the right platform. If you have not configured it explicitly in the config, DBAL tries to guess it, and that requires connecting to the server.

However, I don't see any reason for using MySQL (unless your project does weird thing changing the database config at runtime to move it from mysql to postgresql, and the code now connects before your magic.

@Ocramius I'm in favor of reverting the bugfix and deprecating the feature entirely. Marking a type as commented should be done in the Type class itself rather than in the bundle config (DBAL supports marking a type as commented in the Type itself since 2.4)

sebastianfeldmann pushed a commit to sebastianfeldmann/DoctrineBundle that referenced this issue Jun 29, 2017

Sebastian Feldmann
Revert "Fix an issue were some commented type (doctrine#623)"
This reverts commit 99a62a7.
There where some issues with the database driver configuration.
You can follow the discussion here:
doctrine#673

sebastianfeldmann added a commit to sebastianfeldmann/DoctrineBundle that referenced this issue Jun 29, 2017

Revert "Fix an issue were some commented type (doctrine#623)"
This reverts commit 99a62a7.

There was an issue with the database driver configuration.
You can see the whole discussion here:
doctrine#673
@sebastianfeldmann

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2017

Hi the problem ist not with @mikeSimonson changes introduced with commit 99a62a7

Applying the commented types even if no "un"commented types exist is a valid fix.

Analysis

After consulting wit @Ocramius the assumption is, that because now CommentedTypes are always registered, doctrine tries to register them on the predefined default_connection.

bundle

The problem is that we are now registering types on a connection we don't use, or in this case isn't woking at all.

Solution

A solution would be to handle the type registration within DBAL but because of this breaking backwards compatibility the only thing we can do right now is to throw some more descriptive exception so the user understand the problem.

@Ocramius

This comment has been minimized.

Copy link
Member

commented Jun 29, 2017

Amazing analysis, thanks @sebastianfeldmann!

Closing this, as the original patch WAS correct.

@Majkl578 the problem is that your "generic connection" service still exists and is used in some services, even though the connection was never established before.

@Ocramius Ocramius closed this Jun 29, 2017

@Ocramius Ocramius self-assigned this Jun 29, 2017

sebastianfeldmann added a commit to sebastianfeldmann/DoctrineBundle that referenced this issue Jun 29, 2017

Enhance exception message
The error occures if doctrine needs to establish a connection to
determine the platform version to register the types.
The idea is to provide some more information so the user can
understand what and why this is happening.

This is discussed at:
doctrine#673
@Majkl578

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2017

@sebastianfeldmann Good job there. In other words, 99a62a7 just exposed the issue which was just hiding there previously... And now this is marked as an expected BC break in bugfix version...

Anyway, this behavior doesn't make any sense IMHO. I'm explicitly registering only one connection and default_connection points to this specific connection, so why is the bundle even trying to create a connection that was not requested or even used anywhere? (Also driver defaulting to pdo_mysql is just another crazy assumption, but that's different story.) 😕

So what should I change to fix this then?

@Ocramius

This comment has been minimized.

Copy link
Member

commented Jun 29, 2017

why is the bundle even trying to create a connection that was not requested or even used anywhere?

Usually something referencing the service in a hardcoded way

sebastianfeldmann added a commit to sebastianfeldmann/DoctrineBundle that referenced this issue Jul 1, 2017

Enhance exception message
The error occures if doctrine needs to establish a connection to
determine the platform version to register the types.
The idea is to provide some more information so the user can
understand what and why this is happening.

This is discussed at:
doctrine#673

sebastianfeldmann added a commit to sebastianfeldmann/DoctrineBundle that referenced this issue Jul 1, 2017

Enhance exception message
The error occures if doctrine needs to establish a connection to
determine the platform version to register the types.
The idea is to provide some more information so the user can
understand what and why this is happening.

This is discussed at:
doctrine#673

sebastianfeldmann added a commit to sebastianfeldmann/DoctrineBundle that referenced this issue Jul 2, 2017

Enhance exception message
The error we want to improve occures if doctrine needs to establish
a connection to determine the platform version to register the types.
The idea is to provide some more information so the user can
understand what and why this is happening.

You can follow up on the discussions here:
doctrine#673
doctrine#677

Attention:
The composer doctrine/dbal dependency had to be updated to "^2.5.12".
@JustBlackBird

This comment has been minimized.

Copy link

commented Aug 18, 2017

@Majkl578 Had you finally traced the reason why doctrine.dbal.default_connection is created? I have the same issue here and I almost sure the app does not require doctrine.dbal.default_connection service directly.

@Ocramius Is there any chance doctrine.dbal.default_connection is required somewhere inside of doctrine by some reasons?

StijnVrolijk added a commit to forkcms/forkcms that referenced this issue Feb 9, 2018

Catch Doctrine's DBALException of the database connection fails
On step 4 of the installer, the database variables are guessed from the URL of the website.
Fork then tries to connect to this database to see if it worked. If it doesn't it catches a ConnectionException but since DoctrineBundle 1.6.8 it also throws a DBALException if it can't determine the server version. We now catch that exception as well and treat it like a ConnectionException.

For more information see doctrine/DoctrineBundle#673
@cris2018cris

This comment has been minimized.

Copy link

commented May 15, 2018

a question i try to move symfony to heroku; in local works fine but when i deplay on heroku i found this error ...
what is the solution?

thanks

In ConnectionFactory.php line 96:
                                                                                      
         An exception occured while establishing a connection to figure out your pla  
         tform version.                                                               
         You can circumvent this by setting a 'server_version' configuration value    
                                                                                      
         For further information have a look at:                                      
         https://github.com/doctrine/DoctrineBundle/issues/673 
@cris2018cris

This comment has been minimized.

Copy link

commented May 15, 2018

maybe the solution is path: 'php://stderr'

@lutonda

This comment has been minimized.

Copy link

commented Sep 1, 2018

I soved doing like this
#app\config\config.yml

doctrine:
    dbal:
           default_connection: prod
        connections:
             prod:
                  server_version: 9.4
                  driver: pdo_mysql
                  host: "%database_host%"
                  port: "%database_port%"
                  dbname: "%database_name%"
                  user: "%database_user%"
                  password: "%database_password%"
                  charset: UTF8
                  mapping_types:
                      uuid_binary: binary```
@pfleu

This comment has been minimized.

Copy link

commented Sep 12, 2018

@lutonda Thanks for your solution ! How did you get the server_version value ?

@lutonda

This comment has been minimized.

Copy link

commented Sep 13, 2018

I'm using debian, just run
mysql -V

@aguidis

This comment has been minimized.

Copy link

commented Nov 23, 2018

In my case I just specified the server_version and it just worked:

doctrine:
    dbal:
        driver: pdo_mysql
        host: '%database_host%'
        port: '%database_port%'
        dbname: '%database_name%'
        user: '%database_user%'
        password: '%database_password%'
        charset: UTF8
        server_version: '5.7' # Dark magic here

videni pushed a commit to videni/OroBatchBundle that referenced this issue Mar 18, 2019

BAP-12799: Upgrade of the 3rd party dependencies to the latest stable…
…… (#18487)

* BAP-12799: Upgrade of the 3rd party dependencies to the latest stable version
 - updated doctrine
- updated compiler pass to replace factories in child services because default value was changed to entity_managers.[].repository_factory
- added comment update queries to exclude for all db drivers
- fixed mock after changing interface in doctrine
- fixed unit tests
- fixed issue with join appeared after doctrine update
- updated dev.lock for all applications after backmerge master
 - wrapped dql params in QueryBuilderUtil::checkParameter()
- updated queries after doctrine update
- updated tests after doctrine update with bugfixes
-  enabled schema test, updated comments to sync with database schema
- updated migrations to add missing comments to json fields
 - added db initialization to sh file
- removed dump statement
- added db initialization to js commands check because of doctrine/DoctrineBundle#673

AntoineBvn added a commit to InsaLan/insalan.fr that referenced this issue Apr 9, 2019

@TrafDmitry

This comment has been minimized.

Copy link

commented May 23, 2019

I have same problem with symfony 4 + postgrqs 9.6, "server_version" it work for me, in my doctrine.yaml.Thanks.

tsurowiec added a commit to TheSoftwareHouse/fogger that referenced this issue Jun 11, 2019

@NiklasBr

This comment has been minimized.

Copy link

commented Jul 9, 2019

@aguidis when using your example I encountered the following error:

In DBALException.php line 76:
                                                                               
  Invalid platform version "'5.7'" specified. The platform version has to be   
  specified in the format: "<major_version>.<minor_version>.<patch_version>".  

Changing from this:

server_version: '5.7' # Dark magic here

to this:

server_version: 5.7.0 # Dark magic here

Appears to have fixed my issue.

@justbane

This comment has been minimized.

Copy link

commented Jul 18, 2019

This is a huge problem for me - I am running into this issue on install and server_version is not allowed in the installer.yml (version ~5.8.0)

Where would you identify the server_version in order to run the installer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.