eudoxia0 / crane

An ORM for Common Lisp.
http://eudoxia.me/crane/
201 stars 19 forks source link

Mysql: deftable generates invalid SQL #11

Open eraaij opened 10 years ago

eraaij commented 10 years ago

Hi,

From latest Quicklisp with git pull to recent, trying to do the following against mysql connection:

(deftable users () (user_name :type text) (age :type integer :indexp t))

This gives me the following debug string: Query: CREATE TABLE "users" ( "id" INTEGER AUTO_INCREMENT, "age" INTEGER, "user_name" TEXT, CONSTRAINT "crane_users_id_nullp" CHECK ("id" IS NOT NULL), CONSTRAINT "crane_users_id_primaryp" PRIMARY KEY ("id"));CREATE INDEX "crane_users_age_index" ON "users" ("age");

Mysql does not accept this, instead is happy with:

CREATE TABLE users (id INTEGER AUTO_INCREMENT, age INTEGER, user_name TEXT,
CONSTRAINT crane_users_id_nullp CHECK (id IS NOT NULL),
CONSTRAINT crane_users_id_primaryp PRIMARY KEY (id)) ;CREATE INDEX crane_users_age_index ON users (age);

So it seems that the quotes are in the way for Mysql.

Best, -Emile

jasonmelbye commented 8 years ago

I wonder if this issue is perhaps not related to the quoting. I setup the same deftable form. When I compile it, I receive this error:

DB Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CREATE INDEX "crane_users_age_index" ON "users" ("age")' at line 1 (Code: 1064)

The error I'm receiving does not pop up until the CREATE INDEX part. I think the CREATE TABLE and CREATE INDEX need to be executed separately. If I break that query string up into two parts and execute them separately, both succeed:

CL-USER> (dbi:prepare (crane:get-connection :main) "CREATE TABLE \"users\" ( \"id\" INTEGER AUTO_INCREMENT, \"age\" INTEGER, \"user_name\" TEXT, CONSTRAINT \"crane_users_id_nullp\" CHECK (\"id\" IS NOT NULL), CONSTRAINT \"crane_users_id_primaryp\" PRIMARY KEY (\"id\"));")

DBD.MYSQL:<DBD-MYSQL-QUERY {1007E32543}>

CL-USER> (dbi:execute *)

DBD.MYSQL:<DBD-MYSQL-QUERY {1007E32543}>

CL-USER> (dbi:fetch *) 0 CL-USER> (dbi:prepare (crane:get-connection :main) "CREATE INDEX \"crane_users_age_index\" ON \"users\" (\"age\");")

DBD.MYSQL:<DBD-MYSQL-QUERY {1007F0B8C3}>

CL-USER> (dbi:execute *)

DBD.MYSQL:<DBD-MYSQL-QUERY {1007F0B8C3}>

CL-USER> (dbi:fetch *) 0

An additional note regarding the double quotes: By default, mysql uses the backtick as the quoting character, however, this can be changed to double quotes by setting SQL_MODE to ANSI_QUOTES http://dev.mysql.com/doc/refman/5.5/en/sql-mode.html#sqlmode_ansi_quotes

Crane does this when a mysql connection is created, in set-proper-quote-character (connect.lisp). That code was introduced in May 2014, while this issue was created in July 2014. So I believe the quoting should have been handled correctly. https://github.com/eudoxia0/crane/commit/36aa4ceeb559a3ec31defc8d91697fe7ee2fb557#diff-62d9c383582c11b0993cd0494c128a89

Do you agree the issue can be resolved by executing the CREATE TABLE and CREATE INDEX separately?

jasonmelbye commented 8 years ago

After some additional investigation, it looks like it is possible to execute multiple statements against a mysql database with one call to dbi:execute - but the proper flag needs to be set on the connection.

The mysql client C API allows for flags to be passed in calls to mysql_real_connect() https://dev.mysql.com/doc/refman/5.7/en/mysql-real-connect.html

Of particular interest is the flag (from the table about a 1/3 of the way down the page for mysql_real_connect): CLIENT_MULTI_STATEMENTS Tell the server that the client may send multiple statements in a single string (separated by ; characters). If this flag is not set, multiple-statement execution is disabled. See the note following this table for more information about this flag.

cl-mysql supports this, and in fact, has it set by default if client flags are not passed in. DBI, however, in make-connection passes in a value for :client-flag, so the cl-mysql default is not used.

The easiest solution may be for crane to call dbi:execute one statement at a time. However, it would also be possible to re-work make-connection so that for a mysql connection, that flag is passed in.

eudoxia0 commented 8 years ago

Thanks for this pointer, I'll see where I have to make this change and make sure it stays for the rewrite.