Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The {users}.uid is defined in users.install:
'uid' => array(
'type' => 'serial',
'unsigned' => TRUE,
'not null' => TRUE,
'description' => 'Primary Key: Unique user ID.',
),
(This corresponds to general way of definition of IDs).
In node.install the schema use field:
'uid' => array(
'description' => 'The {users}.uid that owns this node; initially, this is the user that created it.',
'type' => 'int',
'not null' => TRUE,
'default' => 0),
The 'unsigned' => TRUE
is missing by my opinion. This is not probably the only one type mismatch, but I'm not 100% sure this is a bug.
Comment | File | Size | Author |
---|---|---|---|
#18 | core_unsigned_uid_nid.patch | 9.44 KB | Liam Morland |
#17 | core_unsigned_uid_nid.patch | 9.44 KB | Liam Morland |
#16 | core_unsigned_uid_nid.patch | 9.44 KB | Liam Morland |
#14 | core_unsigned_uid_nid.patch | 8.15 KB | Liam Morland |
#8 | core_unsigned_uid_nid.patch | 8.12 KB | Liam Morland |
Comments
Comment #1
Liam MorlandThere are several examples of this in various hook_schema() implementations. The attached patch fixes this in D8 by adding
to any definitions of uid or nid that lack them.
Comment #2
Liam MorlandComment #3
Liam MorlandMarking #1701822: Column Definitions for Columns in Foreign Key Relationships Do Not Match as duplicate.
Comment #4
Crell CreditAttribution: Crell commentedI'm still not sold on directly leveraging foreign keys in core, but our definitions should at least be consistent so that they could be used.
Comment #5
catchThis needs an update from Drupal 7..
Comment #6
Liam MorlandWhere should such updates go? Do they go in core/includes/update.inc or in a system module update hook implementation?
Comment #7
Crell CreditAttribution: Crell commentedSchema-changing updates commands go in the .install file of the appropriate module. In this case, you'd have a couple of update hooks, one for each module that you're touching, that updates that module's tables. See http://api.drupal.org/api/drupal/core!modules!system!system.api.php/func...
Comment #8
Liam MorlandAttached patch includes update functions.
Comment #9
Crell CreditAttribution: Crell commenteddrupal_get_schema() is not reliable within an update function, unfortunately. It uses cached values, not what's in the hook_schema() call. You have to just duplicate the information. :-(
Comment #10
Liam MorlandEven if $rebuild is set? That is supposed to bypass the cache.
Comment #11
Crell CreditAttribution: Crell commentedUnless that changed and I didn't hear about it, there's some totally wacky race conditions involved between caching, modules being enabled, and the environment you're in. Just don't trust hook_schema or drupal_get_schema() while inside an update function.
Comment #12
Liam MorlandIn e.g. comment_update_8002() can I call comment_schema() directly? I can't see how any caching could interfere with that and they are both in the same PHP file, so the function is sure to be there.
Comment #13
Crell CreditAttribution: Crell commentedThat should work, but I think it's discouraged. Catch? (Your call.)
Comment #14
Liam MorlandThe attached patch calls the various schema functions directly. Both those and the update functions are in the same file and the schema functions do not call any other functions, so I cannot see how they could fail to return the correct information.
Comment #15
BerdirCalling the schema functions is wrong, not just discouraged.
Imagine that at a later time, the schema of those tables is changed in a way that is incompatible with this function. For example when the comment entity schema is changed to a multilingual one with two tables. Then the existing update function will fall apart because the structure it expects isn't there.
Comment #16
Liam MorlandThanks. Make sense. Here is a version that duplicates the relevant schema data in the update hook.
Comment #17
Liam MorlandUpdated patch rolled against latest D8.
Comment #18
Liam MorlandReroll.
Comment #19
Crell CreditAttribution: Crell commentedThis looks OK on visual inspection. I don't have time to manually test the upgrade tests right now. If those pass, then this is RTBC.
Comment #20
BerdirLooks good to me too, tests passed.
Comment #21
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks!
Comment #22
frankye CreditAttribution: frankye commented#18: core_unsigned_uid_nid.patch queued for re-testing.
Comment #24
Liam MorlandThe patch failed testing because it has already been committed.