Closed (fixed)
Project:
User Relationships
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
28 Jan 2009 at 17:28 UTC
Updated:
21 May 2009 at 23:20 UTC
Jump to comment: Most recent file
I was looking through the code and it occurred to me that the user_relationships table is not using a SERIAL type column for the rid (relationship ID). The other UR tables that have a unique incrementing ID column are all using SERIAL and the Drupal 6 schema API suggests using SERIAL columns as the old sequences table is deprecated in Drupal 6.
The rid is being determined by this code:
$relationship->rid = db_result(db_query("SELECT MAX(rid) FROM {user_relationships}")) + 1;
That's not necessarily going to be safe if there is a race condition not to mention that it's an
unneeded extra query that wouldn't be needed with a MySQL auto_increment or PostgreSQL serial
column.
Changing the column would require and update function and some testing but other than that I don't see where it would be a problem to implement. What do you think alex.k?
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | user_relationships_api.365623_3.patch.txt | 4.44 KB | alex.k |
| #3 | user_relationships_api.365623_2.patch.txt | 1.81 KB | alex.k |
| #2 | user_relationships_api.365623.patch | 3.6 KB | jaydub |
Comments
Comment #1
alex.k commented@jaydub I agree, without transactions it's a dangerous spot. Glad you noticed. I can test any change here, at least in MySQL-land. Thanks!
Comment #2
jaydub commentedOk how about this patch.
tested the update in PostgreSQL 8.3 successfully. Tested adding a new relationship successfully.
Comment #3
alex.k commentedUpdate did not go through because rid is not actually a unique ID... Two-way relationships use the same rid :-/ just different requester_id and requestee_id.
Tweaked the patch to create just an index, not a unique index - please give it a try. It worked through update and through a new install. Adding/removing relationships worked.
Comment #4
alex.k commentedThere is an issue with this setup. drupal_write_record() will not use the value for rid, even when given, because it's serial. So when inserting the second entry of a two-way relationship the rid will not be the same. The line in user_relationships_save_relationship() that uses drupal_write_record() to do it will need to be rewritten as "INSERT INTO ... SELECT ..." to preserve the same rid.
Comment #5
alex.k commentedThis updated patch replaces drupal_write_record with an INSERT so that rids are reused. I also tweaked indexes definition in hook_schema, but drupal_write_record doesn't use that info when constructing an insert statement.
Comment #6
alex.k commentedCommitted the patch. Tried upgrading existing production sites (mysql), it worked. Please try on PostgreSQL when you have a chance. Thanks!
Comment #8
bhavinshah commentedHi Jaydub,
Am new to drupal and need to know one thing about user relationship. Why rid in table user_relationships is same for both the tuples in when a friend request is being accepted by the user. Any specific reason for that. Please explain, as I need to migrate a huge records to the user_relationships table am finding it difficult to do so...
Comment #9
jaydub commentedSee the answer in #3.
Basically there are two rows for two-way relationships as each user is both a requester and a requestee but there is one relationships and so one relationship ID (rid). For further clarification you'd have to ask alex.k.
Comment #10
alex.k commentedThanks, @jaydub. The reason is database efficiency - when looking for all related users you do not need to do two queries, or a cumbersome OR in the query. So, it is a poor-man's database 'view'. Hence rid staying the same for both records, and unique key spanning rid and the two user IDs