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

Existing columns not found (at least in mysql) #349

Conversation

lucasvanlierop
Copy link
Contributor

Existing columns in the database were not found since dsn was being used instead of just the db name part of the dsn.
Two examples where I ran into this

  • Schema tool trying to create existing columns over and over again.
  • Migrations not being able to set a primary key for columns created earlier.

Note that this fix is quite basic it might the case the conversion has to happen earlier on in the code for example by adding a 'getDatabaseName()' method to \Doctrine\DBAL\Connection or maybe just apply it in \Doctrine\DBAL\Platforms\MySqlPlatform::getListTableIndexesSQL()?

Please let me know

…ame was being as database name instead of just the dbname part
@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-570

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

@@ -155,6 +155,12 @@ public function listTableColumns($table, $database = null)
$database = $this->_conn->getDatabase();
}

// If an entire DSN is passed extract db name from it
preg_match('/dbname=([^;]*)/', $database, $matches);
Copy link
Member

Choose a reason for hiding this comment

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

Why would you want to pass a DSN to this method when a database name is expected?

@lucasvanlierop
Copy link
Contributor Author

Well I don't want to (and thus I don't) but that is what the default value becomes when you not pass a database name to this method, see: $this->_conn->getDatabase().

@deeky666
Copy link
Member

As far as I can see Connection::getDatabase() calls the driver's getDatabase() method which in turn either returns the dbname connection parameter or performs a SELECT DATABASE() query (MySQL). How can a DSN be returned here by the driver? This seems odd. Is there maybe something wrong with your connection parameters? Looks like the DSN is stored in the dbname connection parameter of Doctrine in your case as I cannot imagine MySQL returns a DSN by querying DATABASE().

@lucasvanlierop
Copy link
Contributor Author

Good point @deeky666,

Our connection is created in a subsystem of a bigger project which uses PDO. It turned out Doctrine was being fed a PDO dsn string when connection was created like: 'mysql:host=localhost;dbname={dbName}'. The thing is that Doctrine actually works fine with this configuration except for the issue mentioned above. I will split the configuration into separate host and dbname parameters before passing it to doctrine.

Thanks

@deeky666
Copy link
Member

@lucasvanlierop I'm sure there will be other issues whenever the dbname parameters is needed by the platform to receive information about the schema. Additionally I wonder how you can successfully establish a connection with PDO if the dbname is a DSN. How can any statement work in this case? However, if you can fix your connection string everything should work as expected. You can close the PR if your problem is solved. :)

@lucasvanlierop
Copy link
Contributor Author

Well that apparently most stuff still works when a dsn is passed, that's why it took me some time to discover what was wrong, I'll close this PR now

@guilhermeblanco
Copy link
Member

As per user request...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants