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

[3.x] Get database server version from database so it works also with ProxySQL #312

Draft
wants to merge 6 commits into
base: 3.x-dev
Choose a base branch
from

Conversation

richard67
Copy link
Contributor

@richard67 richard67 commented Sep 8, 2024

Pull Request for Issue #309

Summary of Changes

This pull request (PR) changes the getVersion methods of the "MySQLi" and "MySQL (PDO)" drivers so that they select the version from the database after the first connection instead of relying on the connection's server_info in case of "MySQLi" or PDO::ATTR_SERVER_VERSION in case of "MySQL (PDO)".

The result is cached in a protected class variable so that the database query is only executed at the very first connection of the particular driver instance.

This makes it possible to use ProxySQL for a Joomla 5.x installation.

Testing Instructions

With ProxySQL see issue #309 .

Otherwise (without ProxySQL) check that the version returned by getVersion() is the same as without this PR for the same database, e.g. something like "10.11.7-MariaDB-log" for MariaDB or "8.0.39-0ubuntu0.22.04.1" for MySQL databases.

Documentation Changes Required

None.

@@ -262,9 +270,11 @@ public function connect()
$this->select($this->options['database']);
}

$this->mariadb = stripos($this->connection->server_info, 'mariadb') !== false;
$serverVersion = $this->getVersion();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not happy with this call on every connect, can we lazy load the mariadb and utf8mb4 variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be out of scope of my PR, I think. The change here is equal to what we already have in the MySQL PDO driver.

@richard67
Copy link
Contributor Author

Changing to draft due to review comments until I have a better solution.

@richard67 richard67 marked this pull request as draft October 16, 2024 17:32
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.

2 participants