In Drupal 6 there is a function in the database layer to return the server version - http://api.drupal.org/api/function/db_version/6

This disappeared for Drupal 7 - let's add back a simple method to report this, which is necessary to enable requirements checks against server versions that are also removed (regressed) from Drupal 6: #875674: Report DB version in Status report.

CommentFileSizeAuthor
#9 878198-database-version.patch2.28 KBdamien tournoud
#7 878198-db-version-7.patch1.91 KBpwolanin
#3 878198-db-version-3.patch2.24 KBpwolanin
#2 878198-db-version-2.patch2.21 KBpwolanin

Comments

damien tournoud’s picture

damien tournoud commented

This is not actually necessary for the other patch (the requirements are per-database engine), but I guess it makes sense to restore that.

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new 878198-db-version-2.patch2.21 KB

Here's a simple patch - mostly untested.

pwolanin’s picture

pwolanin commented
Title: Regressions: add back a method to the DB driver interface that reports the server version. » Regression: add back a method to the DB driver interface that reports the server version.
StatusFileSize
new 878198-db-version-3.patch2.24 KB

Fixed mysql query to use proper place holder based on feedback from Damien.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Reviewed & tested by the community

This patch makes me all warm and fuzzy inside.

Crell’s picture

Crell commented

I guess unit tests would be rather silly here. :-) My only question is whether db_version() is still needed, since the only time we'll really be checking the version will be either internal to the DB system or in places where we need to know the type as well, and therefore we'll have the raw connection object anyway.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

I'd be OK with killing db_version.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new 878198-db-version-7.patch1.91 KB

ok, even simpler.

dries’s picture

dries commented
Status: Needs review » Fixed

Thanks, Peter. Committed to CVS HEAD.

damien tournoud’s picture

damien tournoud commented
Status: Fixed » Needs review
StatusFileSize
new 878198-database-version.patch2.28 KB

Reopening this.

PDO actually mandates all database drivers to implement the PDO::ATTR_SERVER_VERSION attribute which gets you the version of the database engine PDO is connected to. We should definitely use that. Verified that this leads to the same result on MySQL and SQLite.

Crell’s picture

Crell commented

As soon as we confirm this works on Postgres I am +1. Damien, if you can verify that go ahead and set it RTBC.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

In addition, we used to report the database version on the status reports page via http://api.drupal.org/api/function/system_requirements/7 -- which was quite handy.

I see in the above that db_version() was purposively removed, but may I ask to revert that? db_version() was a nice little fast way to quickly retrieve that info, without requiring expert DBTNG skills.

pwolanin’s picture

pwolanin commented

@sun I think #875674: Report DB version in Status report. is where I was planning to add it back to the status report.

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

+1 on #9. Nice cleanup. The UI changes belong in the issue that pwolanin linked.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

#9: 878198-database-version.patch queued for re-testing.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Nice clean-up indeed. Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.