Nodapi validate creates duplicates in og2list_groups
joshk - January 5, 2007 - 22:40
| Project: | Organic groups list manager |
| Version: | HEAD |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
Description
In og2list_nodeapi, on validate, we're doing this:
<?php
if (isset($node->recipient) && strlen($node->recipient) && $node->nid > 0) {
db_query("INSERT INTO {og2list_groups} (nid, recipient, status) VALUES (%d, '%s', 1)", $node->nid, $node->recipient);
}
?>But this makes for duplicate og2list_groups records when you edit an existing group. This breaks the incoming mail perlscript. We should pobably not have this insert here, and also it might be good to index the table to prevent duplicate entries.
#1
removed the validate insert for now. Leaving active to look at adding index to table.
#2
This is definitely a problem in the wild, I just removed the duplicate rows for our og2list installation and finally it is working properly.
As for why this is failing... the issue is with the second SQL subquery in mail-in.pl
The duplicate rows break the mail-in.pl script because it has a poorly written query in $add_grp_stmt:
$add_group_stmt = <<EOF;INSERT INTO og2list_incoming_groups
SET mid=(SELECT mid FROM og2list_incoming_content
WHERE msgid=?), oid=(SELECT nid FROM og2list_groups
WHERE recipient=SUBSTRING_INDEX(?, '\@', 1));
EOF
The second subquery (oid= ... ) will fail if more than one row is found with recipient matching. Therefore recipient should be a unique primary key on the og2list_groups table! This query should be writen differently, at the very least the SELECT nid FROM og2list_groups ... should read SELECT max(nid) AS nid FROM og2list_groups ..., and that way if there are multiple rows at least ONE of them comes back. Using max(nid) ensures that if there are two rows and one has a 0 (this does happen as it was a problem in our instance) then you will get the nonzero row.
$add_group_stmt = <<EOF;INSERT INTO og2list_incoming_groups
SET mid=(SELECT mid FROM og2list_incoming_content
WHERE msgid=?), oid=(SELECT max(nid) AS nid FROM og2list_groups
WHERE recipient=SUBSTRING_INDEX(?, '\@', 1));
EOF
A primary key on the og2list_groups table for recipient seems the best way to solve the problem of the double insert while remaining compatible with older MySQL versions.
A better way would be to extend the nodeapi validate query so it does not insert if a matching row is already there, which in mysql seems to require two queries - there is no using NOT EXISTS without a FROM clause, which would allow you to do it in one query. So, you need a transaction to do the existence check and the insert in one opreation, which is probably not compatible enough to be used in this case? or if transactions work in all Mysql's supported then it would be a better way for sure, I am still quite new to MySQL's quirks and have no idea how well stuff like transactions behaves in old versions.
I would be happy to produce a patch implementing these two fixes if it would be useful but it is pretty straightforward. Just adding the max() to the mail-in.pl query will do wonders for people who can't understand why it's not working.