Problem/Motivation
If a field (MySQL) is 'NOT NULL' it must have a 'DEFAULT VALUE', but 'text' and 'blob' types can't have a 'DEFAULT VALUE' but they can be 'NOT NULL'. Database layer has a workaround for 'NOT NULL' fields: first it creates the field with 'NULL DEFAULT VALUE NULL' then alters the field to 'NOT NULL'.
If there are some records already in the database MySQL will throw a warning, that the default 'NULL' value was truncated, and because of this MySQL warning the database layer throws a fatal error.
(This is true for other data types too, but they can have 'DEFAULT VALUE' witch resolves the problem while 'text' and 'blob' can't has.)
I've attached a test only patch, to reproduce the bug, or here is a PHP script witch reproduce it, run it with 'drush scr':
// Just to be sure...
db_drop_table('test_table');
$field = array(
'type' => 'text',
'not null' => TRUE,
);
$table = array(
'fields' => array(
'field_1' => $field,
),
);
// It will be OK
db_create_table('test_table', $table);
print "Table created\n";
// Field will be added.
db_add_field('test_table', 'field_2', $field);
print "New field added\n";
db_insert('test_table')
->fields(array(
'field_1' => 'hello',
'field_2' => 'hello',
))
->execute();
print "Row inserted\n";
// Field will be added but throws a fatal error because the already existing record will be truncated.
db_add_field('test_table', 'field_3', $field);
print "New field added\n";
db_drop_table('test_table');
Proposed resolution
I've attached a patch to resolve the problem: For 'text' and 'blob' types it don't add as NULL and then change it, just add it as 'NOT NULL' without 'default value'.
The patch also contains simpletests.
| Comment | File | Size | Author |
|---|---|---|---|
| #33 | reroll_diff_20-33.txt | 2.78 KB | ravi.shankar |
| #33 | 1747358-33.patch | 1.89 KB | ravi.shankar |
| #20 | set_initial_by-1747358-20.patch | 1.85 KB | alvar0hurtad0 |
| #20 | interdiff.txt | 623 bytes | alvar0hurtad0 |
| #18 | set_initial_by-1747358-18.patch | 1.9 KB | alvar0hurtad0 |
Comments
Comment #0.0
Désiré commentedjust a typo
Comment #1
damien tournoud commentedThis is what
'initial'is for. If you want to create aNOT NULLtable you *have* to give it either a'default'or an'initial'.(By the way, I'm not quite sure how your patch work. Does MySQL really accept adding a
NOT NULLfield without a default value to a table with existing records?)Comment #2
damien tournoud commentedSee the documentation for
db_add_field()for the'initial'key:Comment #3
Désiré commented'initial' is works for, thank you, but in this case, I think for 'NOT NULL' 'text' and 'blob' fields it should be automatically set. I'll modify the patch for do this.
Yes
Comment #4
Désiré commentedComment #5
Désiré commentedComment #6
Désiré commentedComment #8
Désiré commentedtest fixed
Comment #9
Désiré commentedComment #10
damien tournoud commentedI don't see any reason to special case TEXT / BLOB here. True, they cannot have a default value (due to a bug in MySQL), but it's not a reason to force them to an empty string. Doing so would make MySQL even more special in that regard and will increase the portability issues.
In fact, you should *always* set a
'initial'when adding a NON-NULL column. Using'default'to retroactively set the value of existing rows feels like a hack and I don't know if this supported in the databases we support (it's not obvious from the documentations that it would even work).Comment #10.0
damien tournoud commentedtypo
Comment #13
alansaviolobo commentedComment #14
sushilkr commentedRerolled patch
Comment #16
morgantocker commentedJust to clarify the second paragraph of the issue description; in MySQL 5.6 and below, there will be a warning about the truncation when changing a TEXT column that is NULL to NOT NULL. In MySQL 5.7 the behavior will be to error (i.e. refuse the change):
MySQL 5.6 behaviour:
MySQL 5.7 behaviour:
Comment #17
morgantocker commentedThe MySQL Feature Request for DEFAULT support in TEXT/BLOB is http://bugs.mysql.com/bug.php?id=21532
Comment #18
alvar0hurtad0I've unasigned the issue. Very apologies if it is not a good idea.
Comment #20
alvar0hurtad0oups
Comment #32
smustgrave commentedNot sure if this is still relevant but current patch does not apply
Comment #33
ravi.shankar commentedAdded reroll of patch #20 on Drupal 9.4.x. still needs work for the remaining points of #32.