Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Luckily Schema module complains:
* search_dataset.type is part of the primary key but is not specified to be 'not null'.
* search_index.type is part of the primary key but is not specified to be 'not null'.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 853896-remove-default.patch | 1.79 KB | Stevel |
| #6 | 853896-remove-default.patch | 2.4 KB | Stevel |
| search-null-primary-key.patch | 944 bytes | Stevel | |
Comments
Comment #1
jhodgdonThis patch is incomplete - if we do change this, there would also need to be an update function that would alter the fields to be NOT NULL.
That detail aside, I'm not an expert on the details of NOT NULL and primary keys... Obviously, MySQL (and probably PostgreSQL too) must be letting these tables be created with their primary keys and fields as they are, for most people, or we would have had tons of bug reports about this starting in Drupal 6 (the schema has not changed since then, at least in this regard). Why is Luckily Schema complaining?
Comment #2
jhodgdonAh, here is some more information:
http://dev.mysql.com/doc/refman/5.1/en/create-table.html
So, agreed: we should probably change this. Needs a new patch though, since there should be an upgrade function.
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedWe don't actually need the update function if MySQL did that already silently.
Comment #4
Stevel CreditAttribution: Stevel commentedThe fields are effectively created as NOT NULL already because of the primary key definition. This is what creates the mismatch between the schema definition and the actual tables. This also means an update function would not make any change change the database, making it unnecessary. Setting to needs review again.
"Why is Luckily Schema complaining?"
I just mean that it's a good thing that the Schema module complains about this :)
Comment #5
Stevel CreditAttribution: Stevel commentedAha, a field not marked as NOT NULL gets a default of '' (empty string), so we need an update function to remove this default.
Rerolling in an minute...
Comment #6
Stevel CreditAttribution: Stevel commentedNew version of the patch, which modifies the update function that creates the primary key.
Comment #8
Stevel CreditAttribution: Stevel commentedRemoved the ANSI coloring from the patch, no wonder it didn't apply :)
Comment #9
jhodgdonOK. I guess since we are not trying to maintain HEAD to HEAD updates (I think we aren't anyway), editing an existing update function is probably OK. Once we are trying to maintain updates between 7.x versions, of course, we would need to add a new update function to do this.
Regarding the need for an update function in general in this case, I only investigated MySQL, and found that documentation about it silently setting the field to not null = TRUE. I am not sure what happens for PostgreSQL, SQLite, etc. So I do think the update function is a good idea, and I'm glad you had a different reason for thinking it was a good idea too.
Anyway, I guess I'm OK with this patch. Someone else want to RTBC it?
Comment #10
jhodgdon#8: 853896-remove-default.patch queued for re-testing.
Comment #11
jhodgdonThis should be fairly non-controversial...
Comment #12
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.