add_key() in DB Forge is not behaving correctly

[gootik]

The documentation shows that:

$this->dbforge->add_key(array('blog_name', 'blog_label'));
// gives KEY \`blog_name_blog_label\` (\`blog_name\`, \`blog_label\`)

But the below code in add_key() function is preventing this behavior. Because we are never allowed to add an array as a key value the _create_table function will never be picking up an array to add as a key.

if (is_array($key))
{
    foreach ($key as $one)
    {
        $this->add_key($one, $primary);
    }
    return;
}

[GDmac]

Looking at the mysql-driver it does expect key to be either string or an array,
Are you on 3.x develop? If yes, could you do a test leaving that code out? (the whole check for array that is)

EDIT: i'm guessing it should have a test included for the primary key,
in the mysql-driver it implodes the primary_key array,
but for regular keys it checks if a key might be an array itself.
I'm guessing the conditional in DB_forge that you mention probably should read

if ($primary && is_array($key))
{
  // ...
}

[gootik]

I'll write up a test with your suggestion. But you're right that should fix it.

[narfbg]

Now, for the reason I kept this one and #1948 open ... who wants to try and break DB Forge after this commit: a287a34 (do not open unless you want JS formatting thousands of lines of code).

[GDmac]

I made a small forge for the sessions table to test things out. @narfbg

this shows an sql error, the table is created not the less,
and the key (on last_activity) is not set.

https://gist.github.com/4053937

EDIT
additional, i don't see a method to set key names, is there one at all?

EDIT2
i first stuffed this forge into a migration 001 but that causes other issues as well.
e.g. the database failure halts processing, migration number is then not updated,
but the table however was created. (i'll submit a separate issue for the migrate stuff).

[GDmac]

It seems the problem is in the _process_indexes where it returns $sqls (array) by default in DB_forge,
but for the mysqli and mysql forge drivers it returns a $sql string.

[narfbg]

No, that was by design. A property was missing: b0a97c1

On choosing key names - I don't think this was ever possible, the docs don't say anything about it.

[GDmac]

It was never possible.
...but ... If you look at the table definition in the session user_guide,
http://codeigniter.com/user_guide/libraries/sessions.html
you can see the writer knew of its existane
KEY last_activity_idx (last_activity)

edit: if there's a rationale to support named keys, duplicate keys is definitely the main one
http://www.mysqlperformanceblog.com/2008/05/28/should-you-name-indexes-while-doing-alter-table/

[GDmac]

OK, i overlooked that dbforge does add a key name to a key, you can just not name it yourself.
e.g. the key is added as the column name KEY column_name (column_name)

[GDmac]

Am i correct to assume that you can't use dbforge->addkey() after the table has been created?
The user_guide, under add_key() and add_field(), says these 'must' be followed by create_table().

The only way (i see) to add keys to an existing table is to use add_column() with a literal string:
dbforge->add_column('ci_sessions', "KEY ua_key (user_agent)")

Issue:
Creating indexes, when creating a table, is generally handled by the storage engine driver.
But, if you can only ADD indexes or keys with the use of literal strings, (for instance inside Migrations),
doesn't the use of these literal strings defeat the purpose of DB drivers?
e.g. having to add the literal string for all different types of storage engines, instead of letting the driver handle this?

edit: is there a way to drop keys as well? e.g. ALTER TABLE ci_sessionsDROP INDEXua_key;

[narfbg]

Yes, you are correct.
Yes, that is the only way, but you were never supposed to do that, so it doesn't defeat anything.

I don't think that there's a way to drop keys. This as well as adding keys should be considered feature requests.

Edit:

A feature request for adding keys without a create_table() call already exists: #1729