[DC-708] Wrong definition for MySQL string primary column Created: 28/May/10  Updated: 12/Jul/10

Status: Open
Project: Doctrine 1
Component/s: Import/Export
Affects Version/s: 1.2.2
Fix Version/s: 1.2.2

Type: Bug Priority: Major
Reporter: Claudio Nicora Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 0
Labels: None
Environment:

PHP 5.3.1, WinXP-SP3, Doctrine 1.2.2, MySQL 5.1.41

Attachments: File export_mysql_patch.diff    

 Description   

If you define a primary column, the attribute notnull is removed from the column definition because Doctrine assumes that primary columns are always not null.

Now suppose you have a schema like this, with a string primary column.

Client:
  columns:
    serial:    { type: string(50), notnull: true, primary: true }
    name:      { type: string(36), notnull: true }

That's fine, but causes problems with MySQL where the column is created with a default value of "" (empty string) and not <none>.

CREATE TABLE client (serial VARCHAR(50), name VARCHAR(36) NOT NULL, PRIMARY KEY(serial)) ENGINE = INNODB;

Note that the 2nd column is well defined and has <none> as default value (as seen from phpMyAdmin).

I attached a quick-workaround to disable the code which removes the notnull attribute from column definition.

After that the SQL code is like this:

CREATE TABLE client (serial VARCHAR(50) NOT NULL, name VARCHAR(36) NOT NULL, PRIMARY KEY(serial)) ENGINE = INNODB;


 Comments   
Comment by Claudio Nicora [ 28/May/10 ]

Attached a better patch where the notnull attribute is removed only if the primary column type is string

Comment by Claudio Nicora [ 28/May/10 ]

The same behaviour happens even for integer columns, so the notnull attribute should never be removed, not only for string columns. New patch attached, old one removed.

Comment by Jonathan H. Wage [ 08/Jun/10 ]

The patch I see currently just comments out the offending code. Is that intended? It cannot be committed if so

Comment by Claudio Nicora [ 08/Jun/10 ]

My patch only removed the effect, but it's not surely the best solution.

I've no sufficient knowledge on Doctrine to say that commenting out (or removing) that line will not affect other parts; that's why I only commented out the code instead of removing it (both on my side and on the attached patch).

If you think I'm not adding new bugs, I agree that removing the offending code is the cleanest way.

Comment by Jonathan H. Wage [ 08/Jun/10 ]

We can't just remove the code. It will change the behavior of the builder drastically which breaks backwards compatibility.

Comment by Claudio Nicora [ 08/Jun/10 ]

That's what I was afraid of.
Maybe you should change the code that generates the SQL for MySQL to make it include the "NOT NULL" clause to primary keys.

Comment by Jonathan H. Wage [ 08/Jun/10 ]

Hi, if you want to provide a patch that fixes your situation I can test it against our test suite and see if we can include it. You can also run your changes against the test suite to see if it causes any problems.

Comment by Claudio Nicora [ 13/Jun/10 ]

Hi, I attached a patch against Export/Mysql.php instead of the previous against Import/Builder.php.
It works on my side; can you please test it with your test suite?

Generated at Wed Jun 19 10:16:52 UTC 2013 using JIRA 5.2.7#850-sha1:b2af0c8dc8537b36121c6a579fabbdf79fc919e5.