Bugzilla::RelationSet::generateSqlDeltas generates INSERT syntax like this: INSERT INTO watch (watcher, watched) VALUES (83, 1),(83, 77),(83, 3) That, unfortunately, is MySQL-specific. I think the solution is that generateSqlDeltas just shouldn't be used. :-) It's only used for watchers in userprefs.cgi, so we can just get rid of its use there.

Actually, I'll do it. We need this change, locally.

OK, here's the code, without RelationSet being used there. I also added a nice function to Bugzilla::Util, diff_arrays, to facilitate this.

Actually, let's just change the focus of this bug, since it's the last blocker to 281590 and I've already done the hard part.

OK, here we go. Once again, it was pitifully simple and much more efficient to not use RelationSet here.

Comment on attachment 173827 [details] [diff] [review] Remove RelationSet entirely from userprefs >+ my $watched_now = >+ $dbh->selectcol_arrayref("SELECT watched FROM watch" >+ . " WHERE watcher = ?", undef, $userid); What about $old_watch_ids for consistency? "_now" is ambiguous, because I read it as "now that I have updated my list", which is clearly not the case here. >+ # If they're equal, set them to empty. When done, @old contains entries >+ # that were removed; @new contains ones that got added. >+ foreach my $oldv (@old) { >+ foreach my $newv (@new) { >+ next if ($newv eq ''); >+ if ($oldv eq $newv) { >+ $newv = $oldv = ''; >+ } >+ } >+ } Funny way to do this. But I'm not sure it is optimal for long arrays. What about sorting them first so that you don't need to restart from the beginning of the @new array everytime? The rest of the patch is bitrotten and I have absolutely no idea what you do here (this part is completely missing from Util.pm). So please update your patch so that I can review this last part.

(In reply to comment #5) > (From update of attachment 173827 [details] [diff] [review] [edit]) > >+ my $watched_now = > >+ $dbh->selectcol_arrayref("SELECT watched FROM watch" > >+ . " WHERE watcher = ?", undef, $userid); > > What about $old_watch_ids for consistency? "_now" is ambiguous, because I read > it as "now that I have updated my list", which is clearly not the case here. Oh, good idea! I've updated it. :-) > Funny way to do this. But I'm not sure it is optimal for long arrays. What > about sorting them first so that you don't need to restart from the beginning > of the @new array everytime? Because this is exactly the way the old code did it, and I don't want to modify it. You'll notice that I just copied this code out of diff_strings, which is currently working, and rarely has to work on long arrays. Premature optimization is the root of all evil. I fixed the bitrot. :-)

Comment on attachment 175358 [details] [diff] [review] v2 Bye bye RelationSet.pm! :) r=LpSolit

Checking in userprefs.cgi; /cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v <-- userprefs.cgi new revision: 1.68; previous revision: 1.67 done Checking in Bugzilla/Util.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v <-- Util.pm new revision: 1.21; previous revision: 1.20 done