Warning message metatag.revision_id is part of the primary key but is not specified to be 'not null'.

This needs an update script to fix the field for any existing installs.

This may work, I need to test it out.

Updated patch that just includes some drupal_set_message comments.

I tested this on MySQL and it worked, but I don't have PostgreSQL (or anything else) available to test it there.

Committed. Thanks hswong3i!

Actually, 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.

I've reverted this patch and will not be committing it, but I do very much appreciate the time you put into the patch.

I'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

A PRIMARY KEY is a unique index where all key columns must be defined as NOT NULL. If they are not explicitly declared as NOT NULL, MySQL declares them so implicitly (and silently).

For PostGres, from http://www.postgresql.org/docs/9.3/interactive/ddl-constraints.html#DDL-...

Technically, a primary key constraint is simply a combination of a unique constraint and a not-null constraint.

For Oracle, from http://docs.oracle.com/cd/B28359_01/server.111/b28318/glossary.htm#CNCPT...

PRIMARY KEY constraint = Integrity constraint that disallows duplicate values and nulls in a column or set of columns.

MS SQL Server, from http://technet.microsoft.com/en-us/library/aa933112(v=SQL.80).aspx

A table can have only one PRIMARY KEY constraint, and a column that participates in the PRIMARY KEY constraint cannot accept null values.

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.

@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.

This is the most recent patch rerolled.

This 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.

Can you provide a testing sequence or guidance to help move this patch to RTBC.

I 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.

Doing some further consideration of this problem, we need to run an update to ensure all revision_id values are numeric before restructuring the schema.

Committed. FYI the new update is 7016 and the previous one was bumped to 7017.