[aur-dev] [PATCH 2/3] Add "mergepkgid" argument to pkg_delete()

[Dan McGee]

On Tue, Aug 2, 2011 at 7:41 AM, Lukas Fleischer
<archlinux at cryptocrack.de> wrote:
> On Tue, Aug 02, 2011 at 07:13:05AM -0500, Dan McGee wrote:
>> On Sun, Jul 31, 2011 at 12:05 PM, Lukas Fleischer
>> <archlinux at cryptocrack.de> wrote:
>> > This allows for merging comments and votes of deleted packages into
>> > another one which is useful if a package needs to be renamed.
>> >
>> > Signed-off-by: Lukas Fleischer <archlinux at cryptocrack.de>
>> > ---
>> >  web/lib/pkgfuncs.inc.php |   24 +++++++++++++++++++++++-
>> >  1 files changed, 23 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php
>> > index bb5a592..a81ee01 100644
>> > --- a/web/lib/pkgfuncs.inc.php
>> > +++ b/web/lib/pkgfuncs.inc.php
>> > @@ -659,10 +659,11 @@ function pkg_flag ($atype, $ids, $action = True) {
>> >  *
>> >  * @param string $atype Account type, output of account_from_sid
>> >  * @param array $ids Array of package IDs to delete
>> > + * @param int $mergepkgid Package to merge the deleted ones into
>> >  *
>> >  * @return string Translated error or success message
>> >  */
>> > -function pkg_delete ($atype, $ids) {
>> > +function pkg_delete ($atype, $ids, $mergepkgid) {
>> >        if (!$atype) {
>> >                return __("You must be logged in before you can delete packages.");
>> >        }
>> > @@ -678,6 +679,27 @@ function pkg_delete ($atype, $ids) {
>> >        }
>> >
>> >        $dbh = db_connect();
>> > +
>> > +       if ($mergepkgid) {
>> > +               /* Merge comments */
>> > +               $q = "UPDATE IGNORE PackageComments ";
>> Not a fan of this. This is not in any SQL standard so this would be
>> another thing needing adjusted later if someone tried to run using
>> non-MySQL. It is also extremely hacky IMO as you can end up
>> suppressing real errors, and read this gem from the manual:
>>     "Rows for which columns are updated to values that would cause
>> data conversion errors are updated to the closest valid values
>> instead."
>> Which just plain scares me.
>>
>> I'm not even sure why you are doing IGNORE on the comments merge
>> (these can never conflict?), but for the votes merge, simply do a
>> * UPDATE where it doesn't already exist.
>> * DELETE all remaining.
>
> Well, I actually can't remember what I was thinking when I wrote the
> comments query. I probably just copy-pasted it. That's the best excuse I
> can come up with, yeah :)
>
> I'll fix this and think of a better filter criteria for the votes merge
> query. I don't think we need the delete all remaining (duplicate) votes
> though, as we do remove the associated packages anyway and "ON DELETE"
> should do the rest.

Ahh, true on the auto-cascade DELETE thoughts. If you just add a
little more logic to the WHERE clause you should be able to make the
UPDATES selective enough to avoid the duplicates issue.

-Dan