database schema and operations clean up

I need someone with pgsql to test this patch. Just do what stated in the subject, updates database schema, and changes db_query('INSERT to drupal_write_record.

Previous patch didn't include the schema changes needed in the .install file for the drupal_write_record to find the schema.

Thanks deekayen, I guess something went wrong because of the issue #492800: Incorrect table definition, not sure if this is fixed at all..

The error was generated because #2 creates a key id then turns around and drops it on the next line. Why?

Not exactly, line 3 deletes the Index of the id field, not the key.. The problem here is just trying to alter the table (drop index id) with a serial field (id) and not primary key (just dropped in line 1).

Problem was: Previous database schema is wrong..
http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/login_secur...

    'indexes' => array(
      'id' => array('id'),
      'name' => array('name'),
      'host' => array('host')
    ),
    'primary key' => array('timestamp')

1) timestamp can't be primary key, two login attempts can happend at the same timestamp
2) id is indexed, timestamp is not indexed, but queries require tstamp index.

So, the tasks were:
- primary key should be remove from timestamp
- primary key should be created for ID
- index id is deleted
- index timestamp is created.

as shown in the final schema:

    'indexes' => array(
      'name' => array('name'),
      'host' => array('host'),
      'timestamp' => array('timestamp'),
    ),
    'primary key' => array('id')

The howto was:

  // Remove current primary key
  db_drop_primary_key($ret, 'login_security_track');
  // Mysql needs a primary key to perform operations in the table. Create the new one
  db_add_primary_key($ret, 'login_security_track', array('id'));
  // drop the Watch out, it drops the current id INDEX, not the key
  db_drop_index($ret, 'login_security_track', 'id');
  // Create the new index for timestamp
  db_add_index($ret, 'login_security_track', 'timestamp', array('timestamp'));

We can just change the primary key and move the index part to the end as you did, or use the parameter $new_keys in the first alter.

http://api.drupal.org/api/function/db_change_field/6:
"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."

Your code should work (didn't test also, I'll give a try asap) just like this, leaving the db_add_primary_key at the begining..

function login_security_update_6001() {
  $ret = array();
  // step 1, change primary key in the table. 
  db_drop_primary_key($ret, 'login_security_track');
  // mysql needs a key if there's a  a serial field.
  db_add_primary_key($ret, 'login_security_track', array('id'));
  // step 2  remove index for name
  db_drop_index($ret, 'login_security_track', 'name');
  // Not sure if this is already dropped in all db engines because of the primary key drop, so I leave commented
  // db_drop_index($ret, 'login_security_track', 'id');
  db_change_field($ret, 'login_security_track', 'name', 'name', array(
      'type' => 'varchar',
      'length' => 64,
      'not null' => TRUE,
      'default' => '',
      'description' => t("Username used in the login submission."),
    )
  );
  db_change_field($ret, 'login_security_track', 'timestamp', 'timestamp', array(
      'type' => 'int',
      'not null' => TRUE,
      'default' => 0,
      'description' => t("Timestamp of the event."),
    )
  );
  db_add_index($ret, 'login_security_track', 'name', array('name'));
  db_add_index($ret, 'login_security_track', 'timestamp', array('timestamp'));
  return $ret;
}

Note that I just kept the primary key add before the alter. I guess, since we have a serial field, we should db_add_primary_key just inmediatly before the db_drop_primary_key.

In fact, I've tested this:

- Uninstall module
- update login_security module to date 27-Feb-2009 (previous to db changes)
- Intall module
- Update login_security module to 6-1 branch
- Apply this patch
- perform update

And the result is:

The following queries were executed
login_security module
Update #6001

* ALTER TABLE {login_security_track} DROP PRIMARY KEY
* ALTER TABLE {login_security_track} ADD PRIMARY KEY (id)
* ALTER TABLE {login_security_track} DROP INDEX name
* ALTER TABLE {login_security_track} DROP INDEX id
* ALTER TABLE {login_security_track} CHANGE name `name` VARCHAR(64) NOT NULL DEFAULT ''
* ALTER TABLE {login_security_track} CHANGE timestamp `timestamp` INT NOT NULL DEFAULT 0
* ALTER TABLE {login_security_track} ADD INDEX name (name)
* ALTER TABLE {login_security_track} ADD INDEX timestamp (timestamp)

This is the update funtion

function login_security_update_6001() {
  $ret = array();
  // Change current primary key
  db_drop_primary_key($ret, 'login_security_track');
  db_add_primary_key($ret, 'login_security_track', array('id'));
  // Drop indexes
  db_drop_index($ret, 'login_security_track', 'name');
  db_drop_index($ret, 'login_security_track', 'id');
  db_change_field($ret, 'login_security_track', 'name', 'name', array(
      'type' => 'varchar',
      'length' => 64,
      'not null' => TRUE,
      'default' => '',
      'description' => t("Username used in the login submission."),
    )
  );
  db_change_field($ret, 'login_security_track', 'timestamp', 'timestamp', array(
      'type' => 'int',
      'not null' => TRUE,
      'default' => 0,
      'description' => t("Timestamp of the event."),
    )
  );
  // Re-create indexes
  db_add_index($ret, 'login_security_track', 'name', array('name'));
  db_add_index($ret, 'login_security_track', 'timestamp', array('timestamp'));
  return $ret;
}

good!. Thanks for your help and support in the module's queue, deekayen.