Add checkEncoding function in install.inc to ensure collation is correct [#1795900]

The installation instructions state:

The database must be created using a unicode case-insensitive collation (for example the default SQL_Latin1_General_CP1_CI_AS)

Now if one looks at the /includes/database/pgsql/install.inc file one will find a function called checkEncoding() which ensures that the PostGres database is set-up for UTF-8 encoding. Can, and should, something similar be added for sqlsrv to ensure the collation is correct?

Comment	File	Size	Author
#5	_sqlsrv-collation_check-1795900-5.patch	2.01 KB	Uncle_Code_Monkey

Comments

Comment #1

damien tournoud commented 23 February 2013 at 10:29

Yes we can add this check, but someone needs to come up with a patch! :)

Comment #2

rob_johnston commented 26 February 2013 at 14:37

OK... since there may not be any obvious technical limitations, now let's get into the details.

The checkEncoding function for postgres calls "SHOW server_encoding" and expects "UTF8" in return. I believe that for sqlsrv this would be equivalent to "SELECT CONVERT (varchar, SERVERPROPERTY('collation'))" and the response expected is "SQL_Latin1_CP1_CI_AS".

However, I think it would be better to do this for the specific database used for Drupal:
"SELECT CONVERT (varchar, DATABASEPROPERTYEX('" . $database . "','collation'))"
Again, the same response of "SQL_Latin1_General_CP1_CI_AS" is expected.

Whether the database is case insensitive or not is determined by "_CI" or "_CS", but I don't think that there's a good test to determine if the database is unicode or not (isn't this done on the column level?). However, "EXECUTE sp_helpsort" and "SELECT * FROM fn_helpcollations() do give long strings which may or may not contain the work "unicode". So how would unicode support at the database level be determined?

Comment #3

damien tournoud commented 26 February 2013 at 15:25

@rob_johnston: all the types that we are using are Unicode (ie. they are the "n" variants: nvarchar, ntext, etc.). The only thing that need to be checked is if the collation is correctly case insensitive.

Comment #4

rob_johnston commented 26 February 2013 at 21:02

I think that I originally though of this to avoid issues such as what occurred in #1144620: Fix character collation problems

Here's my untested suggestion:

  protected function checkCollation() {
    global $databases;
    
    //REVIEW:  What's the best way to determine the current database?
    $database = $databases['default']['default'];
    $db_name = $database['database']; 
    $sql = "SELECT CONVERT (varchar, DATABASEPROPERTYEX(' . $db_name . ','collation'))";
    
    try {
      $collation = db_query($sql)->fetchField();
      if (stristr($collation, '_CI') !== 'FALSE') {
        $this->pass(st('Database collation is case-insensitive.'));
      }
      else {
        $replacements = array(
            '%driver' => $this->name(),
            '!link' => '<a href="http://drupal.org/project/sqlsrv">Drupal 7 driver for SQL Server and SQL Azure</a>'
        );
        $text  = 'The %driver database must use a case-insensitive collation to work with Drupal.';
        $text .= 'Recreate the database with a case-insensitive collation. See !link for more details.';
        $this->fail(st($text, $replacements));
      }
    }
    catch (Exception $e) {
      $this->fail(st('Drupal could not determine if the collation of the database was set to case-insensitive.'));
    }
  }

Comment #5

Uncle_Code_Monkey commented 19 April 2013 at 20:55

Status:

Active

» Needs review

Status	File	Size
new	_sqlsrv-collation_check-1795900-5.patch	2.01 KB

Patch created after some minor tweaks to the code.

  protected function checkCollation() {
    global $databases;
   
    //REVIEW:  What's the best way to determine the current database?
    $database = $databases['default']['default'];
    $db_name = $database['database'];
    $sql = "SELECT CONVERT (varchar, DATABASEPROPERTYEX('$db_name', 'collation'))";
    try {
      $collation = db_query($sql)->fetchField();
      if (stristr($collation, '_CI') !== FALSE) {
        $this->pass(st('Database collation is case-insensitive.'));
      }
      else {
        $replacements = array(
            '%driver' => $this->name(),
            '!link' => '<a href="http://drupal.org/project/sqlsrv">Drupal 7 driver for SQL Server and SQL Azure</a>',
        );
        $text  = 'The %driver database must use a case-insensitive collation to work with Drupal. ';
        $text .= 'Recreate or modify the database with a case-insensitive collation. See !link for more details.';
        $this->fail(st($text, $replacements));
        $_SESSION['messages']['error'][] = st($text, $replacements);
        return FALSE;
      }
    } catch (Exception $e) {
      $this->fail(st('Drupal could not determine if the collation of the database was set to case-insensitive.'));
      return FALSE;
    }
  }

I added the return FALSE statements to prevent any further processing since the message text seemed to imply it was essential and should stop if the collation was wrong. I also added a $_SESSION message because I couldn't see any text if I visited install.php on an existing database like I had to do. If this isn't proper, feel free to remove it. Anyway, I tested it with my existing database and it passed with a _CI collation and failed when it did not have such a collation.