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

DBAL 3 #8964

Merged
merged 2 commits into from
Aug 31, 2021
Merged

DBAL 3 #8964

merged 2 commits into from
Aug 31, 2021

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Aug 29, 2021

Fixes #8884, #8885.

@derrabus
Copy link
Member Author

I don't understand the failure on Postgres. The test passes locally. 🤔

@greg0ire
Copy link
Member

I do reproduce the failure locally.

docker run -d -p 5432:5432 -e POSTGRES_PASSWORD=postgres postgres:9.6
vendor/bin/phpunit -c ci/github/phpunit/pdo_pgsql.xml   --stop-on-error

I use php 7.4

@greg0ire
Copy link
Member

greg0ire commented Aug 29, 2021

This might be a new issue introduced in doctrine/dbal#4082 … I don't understand why it only affects PostgreSQL though.
Previously, this worked because it resulted in

    public function getDatabase(Connection $conn)
    {
        $params = $conn->getParams();

        return $params['dbname'] ?? $conn->query('SELECT CURRENT_DATABASE()')->fetchOne();
    }

Now it results in

$this->connection = new PDO($dsn, (string) $user, (string) $password, (array) $options);

@greg0ire
Copy link
Member

greg0ire commented Aug 29, 2021

I'm currently working on a fix, we have the database name in $testConnParams in TestUtil

@greg0ire
Copy link
Member

Pushed the fix :)

@derrabus derrabus changed the title WIP: DBAL 3 DBAL 3 Aug 29, 2021
@derrabus
Copy link
Member Author

We're green. The PR is ready, but because I've added a new parameter to various PHPUnit jobs, GitHub is waiting for jobs that don't exist anymore.

@greg0ire
Copy link
Member

Yes. We will have to tweak the settings, and it's going to be a bit complicated because we will have to do so for 2.10.x and 3.x but not for 2.9.x .

@derrabus
Copy link
Member Author

Shall I add dbal-version: ["default"] to 2.9.x as well? It wouldn't do anything there, but it would make the configration easier.

derrabus and others added 2 commits August 29, 2021 21:08
Getting the database name from a connection object results in a PDO
object being created, which might in turn result in an error message if
the database does not exist. For instance it does with PostgreSQL.
In some other situations, like when using sqlite, there is no database
name though, so we still have to fallback on the previous behavior.
@derrabus
Copy link
Member Author

see #8965

@greg0ire greg0ire merged commit 7bf1ad1 into doctrine:2.10.x Aug 31, 2021
@greg0ire
Copy link
Member

Thanks @derrabus ! That was impressive!

@derrabus derrabus deleted the feature/dbal-3 branch August 31, 2021 20:50
@derrabus derrabus added this to the 2.10.0 milestone Sep 6, 2021
This pull request was closed.
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