From 3dddaa3e6ff47b52df5836a49272952893208ddc Mon Sep 17 00:00:00 2001
From: Angie Byron <webchick@24967.no-reply.drupal.org>
Date: Sun, 18 Oct 2009 06:56:24 +0000
Subject: [PATCH] #356074 by chx and Damien Tournoud: Provide a sequences API.
Gets rid of stupid tables that only contain an incrementing ID, and fixes
database import problems due to user ID 0.
---
includes/actions.inc | 2 +-
includes/database/database.inc | 56 +++++++++++++++++
includes/database/mysql/database.inc | 36 +++++++++++
modules/simpletest/tests/actions.test | 6 +-
modules/simpletest/tests/database_test.test | 27 +++++++++
modules/system/system.install | 67 ++++++++++-----------
modules/trigger/trigger.test | 7 ++-
modules/user/user.install | 3 +-
modules/user/user.module | 5 ++
9 files changed, 169 insertions(+), 40 deletions(-)
diff --git a/includes/actions.inc b/includes/actions.inc
index ce196cfe556b..6921e2f44a8c 100644
--- a/includes/actions.inc
+++ b/includes/actions.inc
@@ -324,7 +324,7 @@ function actions_save($function, $type, $params, $label, $aid = NULL) {
// aid is the callback for singleton actions so we need to keep a separate
// table for numeric aids.
if (!$aid) {
- $aid = db_insert('actions_aid')->useDefaults(array('aid'))->execute();
+ $aid = db_next_id();
}
db_merge('actions')
diff --git a/includes/database/database.inc b/includes/database/database.inc
index 82ac78bc7be3..81f04fb2e02d 100644
--- a/includes/database/database.inc
+++ b/includes/database/database.inc
@@ -1047,6 +1047,44 @@ abstract public function mapConditionOperator($operator);
public function commit() {
throw new ExplicitTransactionsNotSupportedException();
}
+
+ /**
+ * Retrieves an unique id from a given sequence.
+ *
+ * Use this function if for some reason you can't use a serial field. For
+ * example, MySQL has no ways of reading of the current value of a sequence
+ * and PostgreSQL can not advance the sequence to be larger than a given
+ * value. Or sometimes you just need a unique integer.
+ *
+ * @param $existing_id
+ * After a database import, it might be that the sequences table is behind,
+ * so by passing in the maximum existing id, it can be assured that we
+ * never issue the same id.
+ * @return
+ * An integer number larger than any number returned by earlier calls and
+ * also larger than the $existing_id if one was passed in.
+ */
+ public function nextId($existing_id = 0) {
+ $transaction = $this->startTransaction();
+ // We can safely use literal queries here instead of the slower query
+ // builder because if a given database breaks here then it can simply
+ // override nextId. However, this is unlikely as we deal with short
+ // strings and integers and no known databases require special handling
+ // for those simple cases.
+ // If another transaction wants to write the same row, it will wait until
+ // this transaction commits.
+ $stmt = $this->query('UPDATE {sequences} SET value = GREATEST(value, :existing_id) + 1', array(
+ ':existing_id' => $existing_id,
+ ));
+ if (!$stmt->rowCount()) {
+ $this->query('INSERT INTO {sequences} (value) VALUES (:existing_id + 1)', array(
+ ':existing_id' => $existing_id,
+ ));
+ }
+ // The transaction gets committed when the transaction object gets
+ // destructed because it gets out of scope.
+ return $new_value;
+ }
}
/**
@@ -2078,6 +2116,24 @@ function db_close(array $options = array()) {
Database::closeConnection($options['target']);
}
+/**
+ * Retrieves a unique id.
+ *
+ * Use this function if for some reason you can't use a serial field,
+ * normally a serial field with db_last_insert_id is preferred.
+ *
+ * @param $existing_id
+ * After a database import, it might be that the sequences table is behind,
+ * so by passing in a minimum id, it can be assured that we never issue the
+ * same id.
+ * @return
+ * An integer number larger than any number returned before for this
+ * sequence.
+ */
+function db_next_id($existing_id = 0) {
+ return Database::getConnection()->nextId($existing_id);
+}
+
/**
* @} End of "defgroup database".
*/
diff --git a/includes/database/mysql/database.inc b/includes/database/mysql/database.inc
index 3f143cfe28b7..bbf927655a75 100644
--- a/includes/database/mysql/database.inc
+++ b/includes/database/mysql/database.inc
@@ -68,6 +68,42 @@ public function mapConditionOperator($operator) {
// We don't want to override any of the defaults.
return NULL;
}
+
+ public function nextId($existing_id = 0) {
+ static $shutdown_registered = FALSE;
+ $new_id = $this->query('INSERT INTO {sequences} () VALUES ()', array(), array('return' => Database::RETURN_INSERT_ID));
+ // This should only happen after an import or similar event.
+ if ($existing_id >= $new_id) {
+ // If we INSERT a value manually into the sequences table, on the next
+ // INSERT, MySQL will generate a larger value. However, there is no way
+ // of knowing whether this value already exists in the table. MySQL
+ // provides an INSERT IGNORE which would work, but that can mask problems
+ // other than duplicate keys. Instead, we use INSERT ... ON DUPLICATE KEY
+ // UPDATE in such a way that the UPDATE does not do anything. This way,
+ // duplicate keys do not generate errors but everything else does.
+ $this->query('INSERT INTO {sequences} (value) VALUES (:value) ON DUPLICATE KEY UPDATE value = value', array(':value' => $existing_id));
+ $new_id = $this->query('INSERT INTO {sequences} () VALUES ()', array(), array('return' => Database::RETURN_INSERT_ID));
+ }
+ if (!$shutdown_registered) {
+ register_shutdown_function(array(get_class($this), 'nextIdDelete'));
+ $shutdown_registered = TRUE;
+ }
+ return $new_id;
+ }
+
+ public static function nextIdDelete() {
+ // While we want to clean up the table to keep it up from occupying too
+ // much storage and memory, we must keep the highest value in the table
+ // because InnoDB uses an in-memory auto-increment counter as long as the
+ // server runs. When the server is stopped and restarted, InnoDB
+ // reinitializes the counter for each table for the first INSERT to the
+ // table based solely on values from the table so deleting all values would
+ // be a problem in this case. Also, TRUNCATE resets the auto increment
+ // counter.
+ $max_id = db_select('SELECT MAX(value) FROM {sequences}')->fetchField();
+ // We know we are using MySQL here, so need for the slower db_delete().
+ db_query('DELETE FROM {sequences} WHERE value < :value', array(':value' => $max_id));
+ }
}
diff --git a/modules/simpletest/tests/actions.test b/modules/simpletest/tests/actions.test
index 12a97cc5fb27..7cc06b558719 100644
--- a/modules/simpletest/tests/actions.test
+++ b/modules/simpletest/tests/actions.test
@@ -37,11 +37,13 @@ class ActionsConfigurationTestCase extends DrupalWebTestCase {
// Make another POST request to the action edit page.
$this->clickLink(t('configure'));
+ preg_match('|admin/config/system/actions/configure/(\d+)|', $this->getUrl(), $matches);
+ $aid = $matches[1];
$edit = array();
$new_action_label = $this->randomName();
$edit['actions_label'] = $new_action_label;
$edit['url'] = 'admin';
- $this->drupalPost('admin/config/system/actions/configure/1', $edit, t('Save'));
+ $this->drupalPost(NULL, $edit, t('Save'));
// Make sure that the action updated properly.
$this->assertText(t('The action has been successfully saved.'), t("Make sure we get a confirmation that we've successfully updated the complex action."));
@@ -51,7 +53,7 @@ class ActionsConfigurationTestCase extends DrupalWebTestCase {
// Make sure that deletions work properly.
$this->clickLink(t('delete'));
$edit = array();
- $this->drupalPost('admin/config/system/actions/delete/1', $edit, t('Delete'));
+ $this->drupalPost("admin/config/system/actions/delete/$aid", $edit, t('Delete'));
// Make sure that the action was actually deleted.
$this->assertRaw(t('Action %action was deleted', array('%action' => $new_action_label)), t('Make sure that we get a delete confirmation message.'));
diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test
index 8d400e68c153..d64ee9ed0fa6 100644
--- a/modules/simpletest/tests/database_test.test
+++ b/modules/simpletest/tests/database_test.test
@@ -2981,3 +2981,30 @@ class DatabaseExtraTypesTestCase extends DrupalWebTestCase {
}
+
+/**
+ * Check the sequences API.
+ */
+class DatabaseNextIdCase extends DrupalWebTestCase {
+ function getInfo() {
+ return array(
+ 'name' => t('Sequences API'),
+ 'description' => t('Test the secondary sequences API.'),
+ 'group' => t('Database'),
+ );
+ }
+
+ /**
+ * Test that the sequences API work.
+ */
+ function testDbNextId() {
+ $first = db_next_id();
+ $second = db_next_id();
+ // We can test for exact increase in here because we know there is no
+ // other process operating on these tables -- normally we could only
+ // expect $second > $first.
+ $this->assertEqual($first + 1, $second, t('The second call from a sequence provides a number increased by one.'));
+ $result = db_next_id(1000);
+ $this->assertEqual($result, 1001, t('Sequence provides a larger number than the existing ID.'));
+ }
+}
diff --git a/modules/system/system.install b/modules/system/system.install
index 97f8e6724624..4e5496f9091c 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -352,23 +352,19 @@ function system_install() {
// Load system theme data appropriately.
system_rebuild_theme_data();
- // Inserting uid 0 here confuses MySQL -- the next user might be created as
- // uid 2 which is not what we want. So we insert the first user here, the
- // anonymous user. uid is 1 here for now, but very soon it will be changed
- // to 0.
db_insert('users')
->fields(array(
+ 'uid' => 0,
'name' => '',
'mail' => '',
))
->execute();
// We need some placeholders here as name and mail are uniques and data is
- // presumed to be a serialized array. Install will change uid 1 immediately
- // anyways. So we insert the superuser here, the uid is 2 here for now, but
- // very soon it will be changed to 1.
-
+ // presumed to be a serialized array. This will be changed by the settings
+ // form.
db_insert('users')
->fields(array(
+ 'uid' => 1,
'name' => 'placeholder-for-uid-1',
'mail' => 'placeholder-for-uid-1',
'created' => REQUEST_TIME,
@@ -376,19 +372,6 @@ function system_install() {
'data' => serialize(array()),
))
->execute();
- // This sets the above two users uid 0 (anonymous). We avoid an explicit 0
- // otherwise MySQL might insert the next auto_increment value.
- db_update('users')
- ->expression('uid', 'uid - uid')
- ->condition('name', '')
- ->execute();
-
- // This sets uid 1 (superuser). We skip uid 2 but that's not a big problem.
- db_update('users')
- ->fields(array('uid' => 1))
- ->condition('name', 'placeholder-for-uid-1')
- ->execute();
-
// Built-in roles.
$rid_anonymous = db_insert('role')
->fields(array('name' => 'anonymous user'))
@@ -615,19 +598,6 @@ function system_schema() {
'primary key' => array('aid'),
);
- $schema['actions_aid'] = array(
- 'description' => 'Stores action IDs for non-default actions.',
- 'fields' => array(
- 'aid' => array(
- 'description' => 'Primary Key: Unique actions ID.',
- 'type' => 'serial',
- 'unsigned' => TRUE,
- 'not null' => TRUE,
- ),
- ),
- 'primary key' => array('aid'),
- );
-
$schema['batch'] = array(
'description' => 'Stores details about batches (processes that run in multiple HTTP requests).',
'fields' => array(
@@ -1451,6 +1421,19 @@ function system_schema() {
'primary key' => array('name'),
);
+ $schema['sequences'] = array(
+ 'description' => 'Stores IDs.',
+ 'fields' => array(
+ 'value' => array(
+ 'description' => 'The value of the sequence.',
+ 'type' => 'serial',
+ 'unsigned' => TRUE,
+ 'not null' => TRUE,
+ ),
+ ),
+ 'primary key' => array('value'),
+ );
+
$schema['sessions'] = array(
'description' => "Drupal's session handlers read and write into the sessions table. Each record represents a user session, either anonymous or authenticated.",
'fields' => array(
@@ -2865,6 +2848,22 @@ function system_update_7043() {
));
}
+/**
+ * Reuse the actions_aid table as sequences.
+ */
+function system_update_7043() {
+ db_drop_primary_key('actions_aid');
+ db_change_field('actions_aid', 'aid', 'value', array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE), array('primary key' => array('value')));
+ db_rename_table('actions_aid', 'sequences');
+ $max = db_query('SELECT MAX(value) FROM {sequences}')->fetchField();
+ $max_uid = db_query('SELECT MAX(uid) FROM {users}')->fetchField();
+ if ($max_uid > $max) {
+ db_update('sequences')->fields(array('value' => $max_uid))->execute();
+ }
+ $max = db_query('SELECT MAX(value) FROM {sequences}')->fetchField();
+ db_delete('sequences')->condition('value', $max, '<');
+}
+
/**
* @} End of "defgroup updates-6.x-to-7.x"
* The next series of updates should start at 8000.
diff --git a/modules/trigger/trigger.test b/modules/trigger/trigger.test
index 7204dc395670..d7732fe02eb9 100644
--- a/modules/trigger/trigger.test
+++ b/modules/trigger/trigger.test
@@ -151,7 +151,10 @@ class TriggerCronTestCase extends DrupalWebTestCase {
'subject' => $action_label,
);
$this->drupalPost('admin/config/system/actions/configure/' . $hash, $edit, t('Save'));
- $edit = array('aid' => md5('1'));
+ $aid = db_query('SELECT aid FROM {actions} WHERE callback = :callback', array(':callback' => 'trigger_test_system_cron_conf_action'))->fetchField();
+ // $aid is likely 3 but if we add more uses for the sequences table in
+ // core it might break, so it is easier to get the value from the database.
+ $edit = array('aid' => md5($aid));
$this->drupalPost('admin/structure/trigger/system', $edit, t('Assign'));
// Add a second configurable action to the cron trigger.
@@ -161,7 +164,7 @@ class TriggerCronTestCase extends DrupalWebTestCase {
'subject' => $action_label,
);
$this->drupalPost('admin/config/system/actions/configure/' . $hash, $edit, t('Save'));
- $edit = array('aid' => md5('2'));
+ $edit = array('aid' => md5($aid + 1));
$this->drupalPost('admin/structure/trigger/system', $edit, t('Assign'));
// Force a cron run.
diff --git a/modules/user/user.install b/modules/user/user.install
index 610d3d566c44..2b93e4e68b37 100644
--- a/modules/user/user.install
+++ b/modules/user/user.install
@@ -102,10 +102,11 @@ function user_schema() {
'description' => 'Stores user data.',
'fields' => array(
'uid' => array(
- 'type' => 'serial',
+ 'type' => 'int',
'unsigned' => TRUE,
'not null' => TRUE,
'description' => 'Primary Key: Unique user ID.',
+ 'default' => 0,
),
'name' => array(
'type' => 'varchar',
diff --git a/modules/user/user.module b/modules/user/user.module
index fe75f1c7b2d3..48140a86247c 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -448,6 +448,11 @@ function user_save($account, $edit = array(), $category = 'account') {
user_module_invoke('update', $edit, $user, $category);
}
else {
+ // Allow 'uid' to be set by the caller. There is no danger of writing an
+ // existing user as drupal_write_record will do an INSERT.
+ if (empty($edit['uid'])) {
+ $edit['uid'] = db_next_id(db_query('SELECT MAX(uid) FROM {users}')->fetchField());
+ }
// Allow 'created' to be set by the caller.
if (!isset($edit['created'])) {
$edit['created'] = REQUEST_TIME;
--
GitLab