Last modified: 2014-04-25 07:07:24 UTC
I think I discovered a problem with the delete wrapper $conds argument. Database::delete documentation says that $conds maybe string|array, but when string is given, it is also treated with makeList( ..., LIST_AND) -- instead treated as plain database statement (e.g. when $conds is manually constructed as explained in [1]) Database::delete lines 2714 seq.: if ( $conds != '*' ) { $sql .= ' WHERE ' . $this->makeList( $conds, LIST_AND ); } $conds needs to be tested for string or array before makeList is applied, in my view. If $conds is a string, then the statement should - in my view - be $sql .= ' WHERE ' . $conds; Shouldn't it ? [1] https://www.mediawiki.org/wiki/Manual:Database_access#Wrapper_function:_select.28.29 "Arguments are either single values (such as 'category' and 'cat_pages > 0') or arrays, if more than one value is passed for an argument position (such as array('cat_pages > 0', $myNextCond)). If you pass in strings, you must manually use DatabaseBase::addQuotes() on your values as you construct the string, as the wrapper will not do this for you. "
Related URL: https://gerrit.wikimedia.org/r/70423 (Gerrit Change Id5a8220d21245669f1091a3b5ed1def65b22d375)
https://gerrit.wikimedia.org/r/70423 (Gerrit Change Id5a8220d21245669f1091a3b5ed1def65b22d375) | change APPROVED and MERGED [by jenkins-bot]
fixed with https://gerrit.wikimedia.org/r/#/c/70423/
Dear core developers, this fix should perhaps be backported to older versions. I never asked for this, so bear with me, if I am wrong...
Remark: the fix is required by https://gerrit.wikimedia.org/r/#/c/69017/
Regarding Backport_WMF: What issues does this actually fix in WMF production?
(In reply to comment #7) > Regarding Backport_WMF: What issues does this actually fix in WMF production? perhaps none, at least, I don't know any - see my "disclaimer" Dear core developers, this fix should perhaps be backported to older versions. I never asked for this, so bear with me, if I am wrong... in comment5 . So this Backport_WMF flag can perhaps be removed, but I feel not competent to decide this.
just saw this on 1.19. Will fix.
Change 80157 had a related patch set uploaded by MarkAHershberger: (bug 50078) Allow a string other than '*' as condition for DatabaseBase::delete() https://gerrit.wikimedia.org/r/80157
Change 80157 merged by MarkAHershberger: (bug 50078) Allow a string other than '*' as condition for DatabaseBase::delete() https://gerrit.wikimedia.org/r/80157
André: just as an example: under certain circumstances, the Extension:UserMerge needs this patch to allow a multi-row delete, and this requires a LIST_OR condition (instead of LIST_AND as default). See https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FUserMerge/c74427a0f6934f877a398a78e579dc96b9d9be12/UserMerge_body.php#L374 . I found the description "Database::delete documentation says that $conds maybe string|array, but when string is given, it is also treated with makeList( ..., LIST_AND) -- instead treated as plain database statement (e.g. when $conds is manually constructed as explained in [1])" wrong when compared to what the core code was actually doing on that date 2013-06-24. So not backporting this means (only) that the extension UserMerge might fail on such wikis.
T.Gries: You set the backport flag 10 months ago so any WMF branches have already superseded any such backport request for 9 1/2 months.
ah, yes
