Make WordPress Core

Opened 7 years ago

Closed 4 years ago

#40037 closed enhancement (fixed)

Add ability to ask wpdb for full db server info

Reported by: clarinetlord's profile clarinetlord Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version: 4.8
Component: Database Keywords: has-patch
Focuses: Cc:

Description

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 7 years ago.
40037.2.diff (1.4 KB) - added by birgire 6 years ago.

Download all attachments as: .zip

Change History (8)

#1 @clarinetlord
7 years ago

  • Keywords has-patch needs-unit-tests added

@birgire
6 years ago

#2 @birgire
6 years 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
5 years 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.

#4 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.5
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#5 @SergeyBiryukov
4 years ago

Previously: #27703. Related: #47738.

#6 @SergeyBiryukov
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 47451:

Database: Introduce wpdb::db_server_info() to retrieve full MySQL server information string as supplied by mysqli_get_server_info().

This complements wpdb::db_version(), which only returns a numeric version string and strips any additional information, e.g. vendor name.

Props clarinetlord, birgire, webaware, pento.
Fixes #40037. See #27703.

Note: See TracTickets for help on using tickets.