| Project: | User Relationships |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | jaydub |
| Status: | closed (fixed) |
Issue Summary
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?
Comments
#1
@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!
#2
Ok how about this patch.
tested the update in PostgreSQL 8.3 successfully. Tested adding a new relationship successfully.
#3
Update 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.
#4
There 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.
#5
This 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.
#6
Committed the patch. Tried upgrading existing production sites (mysql), it worked. Please try on PostgreSQL when you have a chance. Thanks!
#7
Automatically closed -- issue fixed for 2 weeks with no activity.
#8
Hi 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...
#9
See 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.
#10
Thanks, @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
#11
Automatically closed -- issue fixed for 2 weeks with no activity.