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.
schema.module report with following message with latest 7.x-1.x GIT checkout:
- column revision_id - difference on: not null
- declared: array('description' => 'TODO: please describe this field!', 'type' => 'int', 'unsigned' => TRUE, 'not null' => FALSE, 'default' => 0)
- actual: array('description' => 'The entity id revision_id this data is attached to', 'type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0)
Comment | File | Size | Author |
---|---|---|---|
#17 | metatag-n2082539-17.patch | 1.32 KB | DamienMcKenna |
#14 | metatag-n2082539-14.patch | 3.63 KB | DamienMcKenna |
#13 | metatag-n2082539-13.patch | 2.33 KB | DamienMcKenna |
#5 | metatag-n2082539-5.patch | 2.87 KB | DamienMcKenna |
#3 | metatag-n2082539-3.patch | 2.6 KB | DamienMcKenna |
Comments
Comment #1
hswong3i CreditAttribution: hswong3i commentedComment #2
DamienMcKennaThis needs an update script to fix the field for any existing installs.
Comment #3
DamienMcKennaThis may work, I need to test it out.
Comment #4
DamienMcKennaComment #5
DamienMcKennaUpdated patch that just includes some drupal_set_message comments.
Comment #6
DamienMcKennaI tested this on MySQL and it worked, but I don't have PostgreSQL (or anything else) available to test it there.
Comment #7
DamienMcKennaCommitted. Thanks hswong3i!
Comment #8
DamienMcKennaActually, the revision_id field needs to allow NULL values, as e.g. the user entity saves a NULL for the revision_id in all field records.
Comment #9
DamienMcKennaI've reverted this patch and will not be committing it, but I do very much appreciate the time you put into the patch.
Comment #10
rob_johnston CreditAttribution: rob_johnston commentedI'm going to agree with the OP. We had an upgrade on MS SQL Server fail this morning and we fixed it by implement the patch in #1 (and then later found this issue). I've gathered some evidence to support my argument, so here goes...
For MySql, from http://dev.mysql.com/doc/refman/5.7/en/create-table.html
For PostGres, from http://www.postgresql.org/docs/9.3/interactive/ddl-constraints.html#DDL-...
For Oracle, from http://docs.oracle.com/cd/B28359_01/server.111/b28318/glossary.htm#CNCPT...
MS SQL Server, from http://technet.microsoft.com/en-us/library/aa933112(v=SQL.80).aspx
So there are 4 common RDBMS's that are used with Drupal and none of them allow a null value in a primary key. I think the solution here is to not create a primary key, but rather a unique constraint. See db_add_unique_key() function.
Comment #11
rob_johnston CreditAttribution: rob_johnston commentedComment #12
DamienMcKenna@rob_johnston: Thanks for the detailed analysis.
I believe my mistake was examining the a field_data_field_* table rather than field_revision_field_* table - in the former case the table only keeps track of the published revision_id so it isn't part of the primary key, whereas in the latter case there are multiple records and revision_id disallows NULLs and is a primary key.
So yeah, this needs to be applied.
I think the logic around it needs to be verified that a zero is saved as the revision_id.
Comment #13
DamienMcKennaThis is the most recent patch rerolled.
Comment #14
DamienMcKennaThis is an updated version of #13 that updates calls to metatag_metatags_save() so that the revision_id defaults to a zero if there's no actual value.
Comment #15
HyperGlide CreditAttribution: HyperGlide commentedCan you provide a testing sequence or guidance to help move this patch to RTBC.
Comment #16
DamienMcKennaI committed patch #13 instead of #14 as I'm going to leave the fixes for revision_id CRUD logic to #1572474: Add Revisions to Metatags for Entities.
Comment #17
DamienMcKennaDoing some further consideration of this problem, we need to run an update to ensure all revision_id values are numeric before restructuring the schema.
Comment #18
DamienMcKennaCommitted. FYI the new update is 7016 and the previous one was bumped to 7017.