Commit 81256 by goba

#162432 by bjaspan and chx: serial columns need their keys defined in db_change_field() and friends in MySQL

--- includes/database.mysql-common.inc	2007/09/07 10:42:56	1.11
+++ includes/database.mysql-common.inc	2007/09/14 17:46:32	1.12
@@ -1,5 +1,5 @@
 <?php
-// $Id: database.mysql-common.inc,v 1.11 2007/09/07 10:42:56 goba Exp $
+// $Id: database.mysql-common.inc,v 1.12 2007/09/14 17:46:32 goba Exp $
 /**
  * @file
@@ -7,11 +7,6 @@
  */
 /**
- * @ingroup schemaapi
- * @{
- */
-
-/**
  * Runs a basic query in the active database.
  *
  * User-supplied arguments to the query should be passed in as separate
@@ -48,6 +43,11 @@
 }
 /**
+ * @ingroup schemaapi
+ * @{
+ */
+
+/**
  * Generate SQL to create a new table from a Drupal schema definition.
  *
  * @param $name
@@ -71,16 +71,9 @@
   }
   // Process keys & indexes.
-  if (!empty($table['primary key'])) {
-    $sql .= " PRIMARY KEY (". _db_create_key_sql($table['primary key']) ."), \n";
-  }
-  if (!empty($table['unique keys'])) {
-    foreach ($table['unique keys'] as $key => $fields)
-      $sql .= " UNIQUE KEY $key (". _db_create_key_sql($fields) ."), \n";
-  }
-  if (!empty($table['indexes'])) {
-    foreach ($table['indexes'] as $index => $fields)
-      $sql .= " INDEX $index (". _db_create_key_sql($fields) ."), \n";
+  $keys = _db_create_keys_sql($table);
+  if (count($keys)) {
+    $sql .= implode(", \n", $keys) .", \n";
   }
   // Remove the last comma and space.
@@ -91,6 +84,26 @@
   return array($sql);
 }
+function _db_create_keys_sql($spec) {
+  $keys = array();
+
+  if (!empty($spec['primary key'])) {
+    $keys[] = 'PRIMARY KEY ('. _db_create_key_sql($spec['primary key']) .')';
+  }
+  if (!empty($spec['unique keys'])) {
+    foreach ($spec['unique keys'] as $key => $fields) {
+      $keys[] = 'UNIQUE KEY '. $key .' ('. _db_create_key_sql($fields) .')';
+    }
+  }
+  if (!empty($spec['indexes'])) {
+    foreach ($spec['indexes'] as $index => $fields) {
+      $keys[] = 'INDEX '. $index .' ('. _db_create_key_sql($fields) .')';
+    }
+  }
+
+  return $keys;
+}
+
 function _db_create_key_sql($fields) {
   $ret = array();
   foreach ($fields as $field) {
@@ -262,8 +275,15 @@
  *   created field will be set to the value of the key in all rows.
  *   This is most useful for creating NOT NULL columns with no default
  *   value in existing tables.
+ * @param $keys_new
+ *   Optional keys and indexes specification to be created on the
+ *   table along with adding the field. The format is the same as a
+ *   table specification but without the 'fields' element.  If you are
+ *   adding a type 'serial' field, you MUST specify at least one key
+ *   or index including it in this array. @see db_change_field for more
+ *   explanation why.
  */
-function db_add_field(&$ret, $table, $field, $spec) {
+function db_add_field(&$ret, $table, $field, $spec, $keys_new = array()) {
   $fixnull = FALSE;
   if (!empty($spec['not null']) && !isset($spec['default'])) {
     $fixnull = TRUE;
@@ -271,6 +291,9 @@
   }
   $query = 'ALTER TABLE {'. $table .'} ADD ';
   $query .= _db_create_field_sql($field, _db_process_field($spec));
+  if (count($keys_new)) {
+    $query .= ', ADD '. implode(', ADD ', _db_create_keys_sql($keys_new));
+  }
   $ret[] = update_sql($query);
   if (isset($spec['initial'])) {
     // All this because update_sql does not support %-placeholders.
@@ -427,14 +450,49 @@
 /**
  * Change a field definition.
  *
- * IMPORTANT NOTE: On some database systems (notably PostgreSQL),
- * changing a field definition involves adding a new field and
- * dropping an old one. This means that any indices, primary keys and
- * sequences (from serial-type fields) that use the field to be
- * changed get dropped.  For database portability, you MUST drop them
- * explicitly before calling db_change_field() and then re-create them
- * afterwards.  Use db_{add,drop}_{primary_key,unique_key,index} for
- * this purpose.
+ * IMPORTANT NOTE: To maintain database portability, you have to explicitly
+ * recreate all indices and primary keys that are using the changed field.
+ *
+ * That means that you have to drop all affected keys and indexes with
+ * db_drop_{primary_key,unique_key,index}() before calling db_change_field().
+ * To recreate the keys and indices, pass the key definitions as the
+ * optional $keys_new argument directly to db_change_field().
+ *
+ * For example, suppose you have:
+ * @code
+ * $schema['foo'] = array(
+ *   'fields' => array(
+ *     'bar' => array('type' => 'int', 'not null' => TRUE)
+ *   ),
+ *   'primary key' => array('bar')
+ * );
+ * @endcode
+ * and you want to change foo.bar to be type serial, leaving it as the
+ * primary key.  The correct sequence is:
+ * @code
+ * db_drop_primary_key($ret, 'foo');
+ * db_change_field($ret, 'foo', 'bar', 'bar',
+ *   array('type' => 'serial', 'not null' => TRUE),
+ *   array('primary key' => array('bar')));
+ * @endcode
+ *
+ * The reasons for this are due to the different database engines:
+ *
+ * On PostgreSQL, changing a field definition involves adding a new field
+ * and dropping an old one which* causes any indices, primary keys and
+ * sequences (from serial-type fields) that use the changed field to be dropped.
+ *
+ * On MySQL, all type 'serial' fields must be part of at least one key
+ * or index as soon as they are created.  You cannot use
+ * db_add_{primary_key,unique_key,index}() for this purpose because
+ * the ALTER TABLE command will fail to add the column without a key
+ * or index specification.  The solution is to use the optional
+ * $keys_new argument to create the key or index at the same time as
+ * field.
+ *
+ * You could use db_add_{primary_key,unique_key,index}() in all cases
+ * unless you are converting a field to be type serial. You can use
+ * the $keys_new argument in all cases.
  *
  * @param $ret
  *   Array to which query results will be added.
@@ -446,10 +504,19 @@
  *   New name for the field (set to the same as $field if you don't want to change the name).
  * @param $spec
  *   The field specification for the new field.
+ * @param $keys_new
+ *   Optional keys and indexes specification to be created on the
+ *   table along with changing the field. The format is the same as a
+ *   table specification but without the 'fields' element.
  */
-function db_change_field(&$ret, $table, $field, $field_new, $spec) {
-  $ret[] = update_sql("ALTER TABLE {". $table ."} CHANGE $field ".
-    _db_create_field_sql($field_new, _db_process_field($spec)));
+
+function db_change_field(&$ret, $table, $field, $field_new, $spec, $keys_new = array()) {
+  $sql = 'ALTER TABLE {'. $table .'} CHANGE '. $field .' '.
+    _db_create_field_sql($field_new, _db_process_field($spec));
+  if (count($keys_new)) {
+    $sql .= ', ADD '.implode(', ADD ', _db_create_keys_sql($keys_new));
+  }
+  $ret[] = update_sql($sql);
 }
 /**
@@ -463,7 +530,3 @@
 function db_last_insert_id($table, $field) {
   return db_result(db_query('SELECT LAST_INSERT_ID()'));
 }
-
-/**
- * @} End of "ingroup schemaapi".
- */
--- includes/database.pgsql.inc	2007/09/07 10:42:56	1.59
+++ includes/database.pgsql.inc	2007/09/14 17:46:32	1.60
@@ -1,5 +1,5 @@
 <?php
-// $Id: database.pgsql.inc,v 1.59 2007/09/07 10:42:56 goba Exp $
+// $Id: database.pgsql.inc,v 1.60 2007/09/14 17:46:32 goba Exp $
 /**
  * @file
@@ -559,6 +559,22 @@
   return implode(', ', $ret);
 }
+function _db_create_keys(&$ret, $table, $new_keys) {
+  if (isset($new_keys['primary key'])) {
+    db_add_primary_key($ret, $table, $new_keys['primary key']);
+  }
+  if (isset($new_keys['unique keys'])) {
+    foreach ($new_keys['unique keys'] as $name => $fields) {
+      db_add_unique_key($ret, $table, $name, $fields);
+    }
+  }
+  if (isset($new_keys['indexes'])) {
+    foreach ($new_keys['indexes'] as $name => $fields) {
+      db_add_index($ret, $table, $name, $fields);
+    }
+  }
+}
+
 /**
  * Set database-engine specific properties for a field.
  *
@@ -665,8 +681,15 @@
  *   created field will be set to the value of the key in all rows.
  *   This is most useful for creating NOT NULL columns with no default
  *   value in existing tables.
+ * @param $keys_new
+ *   Optional keys and indexes specification to be created on the
+ *   table along with adding the field. The format is the same as a
+ *   table specification but without the 'fields' element.  If you are
+ *   adding a type 'serial' field, you MUST specify at least one key
+ *   or index including it in this array. @see db_change_field for more
+ *   explanation why.
  */
-function db_add_field(&$ret, $table, $field, $spec) {
+function db_add_field(&$ret, $table, $field, $spec, $new_keys = array()) {
   $fixnull = FALSE;
   if (!empty($spec['not null']) && !isset($spec['default'])) {
     $fixnull = TRUE;
@@ -684,6 +707,9 @@
   if ($fixnull) {
     $ret[] = update_sql("ALTER TABLE {". $table ."} ALTER $field SET NOT NULL");
   }
+  if (isset($new_keys)) {
+    _db_create_keys($ret, $table, $new_keys);
+  }
 }
 /**
@@ -831,14 +857,49 @@
 /**
  * Change a field definition.
  *
- * IMPORTANT NOTE: On some database systems (notably PostgreSQL),
- * changing a field definition involves adding a new field and
- * dropping an old one. This means that any indices, primary keys and
- * sequences (from serial-type fields) that use the field to be
- * changed get dropped.  For database portability, you MUST drop them
- * explicitly before calling db_change_field() and then re-create them
- * afterwards.  Use db_{add,drop}_{primary_key,unique_key,index} for
- * this purpose.
+ * IMPORTANT NOTE: To maintain database portability, you have to explicitly
+ * recreate all indices and primary keys that are using the changed field.
+ *
+ * That means that you have to drop all affected keys and indexes with
+ * db_drop_{primary_key,unique_key,index}() before calling db_change_field().
+ * To recreate the keys and indices, pass the key definitions as the
+ * optional $new_keys argument directly to db_change_field().
+ *
+ * For example, suppose you have:
+ * @code
+ * $schema['foo'] = array(
+ *   'fields' => array(
+ *     'bar' => array('type' => 'int', 'not null' => TRUE)
+ *   ),
+ *   'primary key' => array('bar')
+ * );
+ * @endcode
+ * and you want to change foo.bar to be type serial, leaving it as the
+ * primary key.  The correct sequence is:
+ * @code
+ * db_drop_primary_key($ret, 'foo');
+ * db_change_field($ret, 'foo', 'bar', 'bar',
+ *   array('type' => 'serial', 'not null' => TRUE),
+ *   array('primary key' => array('bar')));
+ * @endcode
+ *
+ * The reasons for this are due to the different database engines:
+ *
+ * On PostgreSQL, changing a field definition involves adding a new field
+ * and dropping an old one which* causes any indices, primary keys and
+ * sequences (from serial-type fields) that use the changed field to be dropped.
+ *
+ * On MySQL, all type 'serial' fields must be part of at least one key
+ * or index as soon as they are created.  You cannot use
+ * db_add_{primary_key,unique_key,index}() for this purpose because
+ * the ALTER TABLE command will fail to add the column without a key
+ * or index specification.  The solution is to use the optional
+ * $new_keys argument to create the key or index at the same time as
+ * field.
+ *
+ * You could use db_add_{primary_key,unique_key,index}() in all cases
+ * unless you are converting a field to be type serial. You can use
+ * the $new_keys argument in all cases.
  *
  * @param $ret
  *   Array to which query results will be added.
@@ -850,17 +911,29 @@
  *   New name for the field (set to the same as $field if you don't want to change the name).
  * @param $spec
  *   The field specification for the new field.
+ * @param $new_keys
+ *   Optional keys and indexes specification to be created on the
+ *   table along with changing the field. The format is the same as a
+ *   table specification but without the 'fields' element.
  */
-function db_change_field(&$ret, $table, $field, $field_new, $spec) {
+function db_change_field(&$ret, $table, $field, $field_new, $spec, $new_keys = array()) {
   $ret[] = update_sql("ALTER TABLE {". $table ."} RENAME $field TO ". $field ."_old");
   $not_null = isset($spec['not null']) ? $spec['not null'] : FALSE;
   unset($spec['not null']);
+
   db_add_field($ret, $table, "$field_new", $spec);
+
   $ret[] = update_sql("UPDATE {". $table ."} SET $field_new = ". $field ."_old");
+
   if ($not_null) {
     $ret[] = update_sql("ALTER TABLE {". $table ."} ALTER $field_new SET NOT NULL");
   }
+
   db_drop_field($ret, $table, $field .'_old');
+
+  if (isset($new_keys)) {
+    _db_create_keys($ret, $table, $new_keys);
+  }
 }
 /**
--- modules/block/block.schema	2007/08/19 08:08:44	1.2
+++ modules/block/block.schema	2007/09/14 17:46:32	1.3
@@ -1,5 +1,5 @@
 <?php
-// $Id: block.schema,v 1.2 2007/08/19 08:08:44 dries Exp $
+// $Id: block.schema,v 1.3 2007/09/14 17:46:32 goba Exp $
 function block_schema() {
   $schema['blocks'] = array(
@@ -36,7 +36,7 @@
   $schema['boxes'] = array(
     'fields' => array(
-      'bid'    => array('type' => 'serial', 'not null' => TRUE),
+      'bid'    => array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE),
       'body'   => array('type' => 'text', 'not null' => FALSE, 'size' => 'big'),
       'info'   => array('type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => ''),
       'format' => array('type' => 'int', 'size' => 'small', 'not null' => TRUE, 'default' => 0)
--- modules/system/system.install	2007/09/14 09:43:13	1.152
+++ modules/system/system.install	2007/09/14 17:46:32	1.153
@@ -1,5 +1,5 @@
 <?php
-// $Id: system.install,v 1.152 2007/09/14 09:43:13 goba Exp $
+// $Id: system.install,v 1.153 2007/09/14 17:46:32 goba Exp $
 /**
  * Test and report Drupal installation requirements.
@@ -3052,8 +3052,6 @@
   switch ($GLOBALS['db_type']) {
     case 'pgsql':
-      $ret[] = update_sql("ALTER TABLE {node} DROP CONSTRAINT {node}_pkey");
-      $ret[] = update_sql("ALTER TABLE {node} ADD PRIMARY KEY (nid)");
       $ret[] = update_sql("ALTER TABLE {node} ADD CONSTRAINT {node}_nid_vid_key UNIQUE (nid, vid)");
       db_add_column($ret, 'blocks', 'bid', 'serial');
       $ret[] = update_sql("ALTER TABLE {blocks} ADD PRIMARY KEY (bid)");
@@ -3070,8 +3068,6 @@
       break;
     case 'mysql':
     case 'mysqli':
-      $ret[] = update_sql('ALTER TABLE {node} DROP PRIMARY KEY');
-      $ret[] = update_sql('ALTER TABLE {node} ADD PRIMARY KEY (nid)');
       $ret[] = update_sql('ALTER TABLE {node} ADD UNIQUE KEY nid_vid (nid, vid)');
       $ret[] = update_sql("ALTER TABLE {blocks} ADD bid int NOT NULL AUTO_INCREMENT PRIMARY KEY");
       $ret[] = update_sql("ALTER TABLE {filters} ADD fid int NOT NULL AUTO_INCREMENT PRIMARY KEY");
@@ -3198,12 +3194,14 @@
         db_drop_primary_key($ret, 'term_node');
         db_add_primary_key($ret, 'term_node', array('vid', 'tid', 'nid'));
       }
-
-      // Remove defaults on batch columns.
-      if (db_table_exists('batch')) {
-        db_field_set_no_default($ret, 'batch', 'token');
-        db_field_set_no_default($ret, 'batch', 'timestamp');
-      }
+      
+      // Make boxes.bid unsigned.
+      db_drop_primary_key($ret, 'boxes');
+      db_change_field($ret, 'boxes', 'bid', 'bid', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE), array('primary key' => array('bid')));
+
+      // Fix primary key
+      db_drop_primary_key($ret, 'node');
+      db_add_primary_key($ret, 'node', array('nid'));
       break;
@@ -3235,20 +3233,20 @@
       db_change_field($ret, 'cache_form', 'serialized', 'serialized', array('type' => 'int', 'size' => 'small', 'not null' => TRUE, 'default' => 0));
       // Remove default => 0, set auto increment.
-      db_change_field($ret, 'files', 'fid', 'fid', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE));
-
-      // Remove default => 0, set auto increment.
-      $ret[] = update_sql("SET sql_mode = 'NO_AUTO_VALUE_ON_ZERO'");
-      db_change_field($ret, 'users', 'uid', 'uid', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE));
-
-      // Set auto increment.
-      db_change_field($ret, 'node_revisions', 'vid', 'vid', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE));
-
-      // Set auto increment.
-      db_change_field($ret, 'boxes', 'bid', 'bid', array('type' => 'serial', 'not null' => TRUE));
-
-      // Set auto increment, unsigned.
-      db_change_field($ret, 'batch', 'bid', 'bid', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE));
+      $new_uid = 1 + db_result(db_query('SELECT MAX(uid) FROM {users}'));
+      $ret[] = update_sql('UPDATE users SET uid = '. $new_uid . ' WHERE uid = 0');
+      db_drop_primary_key($ret, 'users');
+      db_change_field($ret, 'users', 'uid', 'uid', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE), array('primary key' => array('uid')));
+      $ret[] = update_sql('UPDATE users SET uid = 0 WHERE uid = '. $new_uid);
+
+      // Special field names.
+      $map = array('node_revisions' => 'vid');
+      // Make sure these tables have proper auto_increment fields.
+      foreach (array('boxes', 'files', 'node', 'node_revisions') as $table) {
+        $field = isset($map[$table]) ? $map[$table] : $table[0] .'id';
+        db_drop_primary_key($ret, $table);
+        db_change_field($ret, $table, $field, $field, array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE), array('primary key' => array($field)));
+      }
       break;
   }
--- update.php	2007/09/02 15:19:16	1.232
+++ update.php	2007/09/14 17:46:31	1.233
@@ -1,5 +1,5 @@
 <?php
-// $Id: update.php,v 1.232 2007/09/02 15:19:16 goba Exp $
+// $Id: update.php,v 1.233 2007/09/14 17:46:31 goba Exp $
 /**
  * @file
@@ -677,30 +677,19 @@
     return;
   }
+  $schema['batch'] = array(
+    'fields' => array(
+      'bid'       => array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE),
+      'token'     => array('type' => 'varchar', 'length' => 64, 'not null' => TRUE),
+      'timestamp' => array('type' => 'int', 'not null' => TRUE),
+      'batch'     => array('type' => 'text', 'not null' => FALSE, 'size' => 'big')
+    ),
+    'primary key' => array('bid'),
+    'indexes' => array('token' => array('token')),
+  );
+
   $ret = array();
-  switch ($GLOBALS['db_type']) {
-    case 'mysql':
-    case 'mysqli':
-      $ret[] = update_sql("CREATE TABLE {batch} (
-        bid int(11) NOT NULL,
-        token varchar(64) NOT NULL,
-        timestamp int(11) NOT NULL,
-        batch longtext,
-        PRIMARY KEY  (bid),
-        KEY token (token)
-      ) /*!40100 DEFAULT CHARACTER SET UTF8 */ ");
-      break;
-    case 'pgsql':
-      $ret[] = update_sql("CREATE TABLE {batch} (
-        bid serial CHECK (bid >= 0),
-        token varchar(64) NOT NULL default '',
-        timestamp int NOT NULL default '0',
-        batch text,
-        PRIMARY KEY (bid)
-      )");
-      $ret[] = update_sql("CREATE INDEX {batch}_token_idx ON {batch} (token)");
-     break;
-  }
+  db_create_table($ret, 'batch', $schema['batch']);
   return $ret;
 }
No votes yet