Double quoting of auto-increment index in mysql/innodb

[Jeff Dairiki]

Example:

from sqlalchemy import *

meta = MetaData()
table = Table('table', meta,
              Column('id', Integer, primary_key=True, autoincrement=False),
              Column('order', Integer, primary_key=True),
              mysql_engine='InnoDB')

def dump(sql, *args, **kwargs):
    print sql.compile(dialect=engine.dialect)

engine = create_engine('mysql://', strategy='mock', executor=dump)

table.create(engine, checkfirst=False)

which produces:

CREATE TABLE `table` (
    id INTEGER NOT NULL, 
    `order` INTEGER NOT NULL AUTO_INCREMENT, 
    PRIMARY KEY (id, `order`), 
    KEY `idx_autoinc_order`(``order``)
)ENGINE=InnoDB

Note the extra quotes around {{{order}}} in the {{{idx_autoinc_order}}} index.

I will attach my guess at a patch.

[Jeff Dairiki]

Diffs on current mecurial tip a98001d03a2f

[Mike Bayer]

it looks like the quoting of reserved word "order" is conflicting with the assumption that the name needs to be quoted unconditionally.

Is it the case here that the quoting is not needed for non-reserved word in:

  KEY `idx_autoinc_id`(some_column)

?

if so, then this is the right patch.

otherwise, if quotes are required unconditionally (also let's consider MySQL 4 versions too), the solution would be to remove the call to preparer.format_column().

If you can check on this that would help a lot ! thanks

[Jeff Dairiki]

In mysql 5.1, at least, the quotes are not needed except around names which are not reserved words. (I just tested this.) (The idx_autoinc_id doesn't need quotes either.)

I'm pretty sure all mysqls are like this, but I don't have a mysql4 handy to test on. From the 4.1 docs:

An identifier may be quoted or unquoted. If an identifier contains special characters or is a reserved word, you must quote it whenever you refer to it. The set of alphanumeric characters from the current character set, “_”, and “$” are not special. Reserved words are listed at Section 8.3, “Reserved Words”. (Exception: A reserved word that follows a period in a qualified name must be an identifier, so it need not be quoted.)

Apparently even back-ticks are legal characters within names, so long as they are quoted. So you can't just wrap the name in backticks — any backticks in the name itself have to be doubled. I haven't checked to see if {{{.preparer.format_column()}}} does that (I'd guess so); but the quoting of the idx_autoinc_id is apparently broken (and my patch does not fix that.) ... :-/

[Mike Bayer]

so we might consider this patch:

diff -r 4cb74452fe551c3d4f0dd305bee1e69dbdccd99a lib/sqlalchemy/dialects/mysql/base.py
--- a/lib/sqlalchemy/dialects/mysql/base.py Thu Apr 05 14:31:28 2012 -0400
+++ b/lib/sqlalchemy/dialects/mysql/base.py Sun Apr 08 10:11:12 2012 -0400
@@ -1395,8 +1395,10 @@
                 auto_inc_column is not list(table.primary_key)[0](0):
             if constraint_string:
                 constraint_string += ", \n\t"
-            constraint_string += "KEY `idx_autoinc_%s`(`%s`)" % (auto_inc_column.name, \
-                            self.preparer.format_column(auto_inc_column))
+            constraint_string += "KEY %s (%s)" % (
+                    self.preparer.quote("idx_autoinc_%s" % auto_inc_column.name, None), 
+                    self.preparer.format_column(auto_inc_column)
+            )

         return constraint_string

or we could go the other way and just quote unconditionally. it's what mysqldump does...

CREATE TABLE `foo` (
  `id` int(11) NOT NULL,
  KEY `idx_autoinc_id` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

but we've gotten this far without unconditional quoting....

[Mike Bayer]

one hour later....

[local](root@f1)# ./bin/mysql
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1 to server version: 4.1.26

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql> use test;
Database changed
mysql> create table foo (id integer, key idx_autoinc_id (id))engine=InnoDB;
Query OK, 0 rows affected (0.03 sec)

mysql> create table bar (id integer, key `idx_autoinc_id` (`id`))engine=InnoDB;
Query OK, 0 rows affected (0.02 sec)

hooray. now i know how to build ancient mysql's too. 4.1.26 is enough for me...

[Mike Bayer]

b7abf86f72e62644846dfd540bcceae7959cd046, 3269b73ff7a12303aadcaed0246d401ec649fa94

[Jeff Dairiki]

Excellent, thanks Mike!

I was pretty sure there was a {{{.preparer.quote()}}}, but I hadn't yet dug far enough to find it.

[Mike Bayer]

Removing milestone: 0.7.7 (automated comment)