Make WordPress Core

Opened 3 years ago

Last modified 13 months ago

#40037 new enhancement

Add ability to ask wpdb for full db server info

Reported by: clarinetlord Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Database Keywords: has-patch
Focuses: Cc:
PR Number:


Currently, calling wpdb::db_version() only returns the version number of the db server. It would be useful to have a separate method called db_server_info() that returned the full string with vendor and version. That way one wouldn't have to use the raw sql function mysqli_get_server_info() outside of wpdb and allows wpdb to properly abstract this functionality.

Attachments (2)

Added_method_to_wpdb_to_get_full_server_info.patch (1.6 KB) - added by clarinetlord 3 years ago.
40037.2.diff (1.4 KB) - added by birgire 23 months ago.

Download all attachments as: .zip

Change History (5)

#1 @clarinetlord
3 years ago

  • Keywords has-patch needs-unit-tests added

23 months ago

#2 @birgire
23 months ago

This enhancement would be useful in #39165 where it currently uses:

if ( method_exists( $wpdb, 'db_version' ) ) {
	if ( $wpdb->use_mysqli ) {
		$server = mysqli_get_server_info( $wpdb->dbh );
	} else {
		$server = mysql_get_server_info( $wpdb->dbh );
} else {
	$server = null;

I refreshed the patch into a new 40037.2.diff, according to the Coding Standard (e.g. use tabs for indentation, added @since etc).

Regarding a "unit test" for the method, I think it's implicitly covered with the existing test method for the db version:

function test_db_version() {
	global $wpdb;

	$this->assertTrue( version_compare( $wpdb->db_version(), '5.0', '>=' ) );

as $wpdb::db_version() would be a wrapper for $wpdb::db_server_info().

Otherwise this kind of test:

function test_db_server_info() {
	global $wpdb;
	$this->assertTrue( is_string( $wpdb->db_server_info() ) );

feels like we're just testing the output type of the PHP server info functions.

#3 @clarinetlord
13 months ago

  • Keywords needs-unit-tests removed

Thanks for the updated patch @birgire. I agree that this probably doesn't need additional unit tests, as it is implicitly covered already.

Note: See TracTickets for help on using tickets.