From 11ebb98919f56f7dcf888adfbebf9e8826f995b4 Mon Sep 17 00:00:00 2001 From: Alexei Sorokin Date: Thu, 27 Aug 2020 11:15:39 +0300 Subject: [PATCH] [DATABASE] Fix use of ORDER BY with DISTINCT statuses/retweets_of_me has performance fixed, so it is also stripped of its "bad query" status. --- .../Share/actions/apitimelineretweetsofme.php | 89 +++++++-------- modules/Share/lib/repeatsofmenoticestream.php | 108 +++++++----------- plugins/Embed/scripts/fixup_files.php | 57 ++++----- .../scripts/removeRemoteMedia.php | 34 +++--- 4 files changed, 130 insertions(+), 158 deletions(-) diff --git a/modules/Share/actions/apitimelineretweetsofme.php b/modules/Share/actions/apitimelineretweetsofme.php index 890c5f4d85..045e8be1c5 100644 --- a/modules/Share/actions/apitimelineretweetsofme.php +++ b/modules/Share/actions/apitimelineretweetsofme.php @@ -1,44 +1,38 @@ . + /** - * StatusNet, the distributed open-source microblogging tool - * * Show authenticating user's most recent notices that have been repeated * - * PHP version 5 - * - * LICENCE: This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * * @category API - * @package StatusNet + * @package GNUsocial * @author Evan Prodromou * @copyright 2009 StatusNet, Inc. - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later */ -if (!defined('STATUSNET')) { - exit(1); -} +defined('GNUSOCIAL') || die(); /** * Show authenticating user's most recent notices that have been repeated * - * @category API - * @package StatusNet - * @author Evan Prodromou - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ + * @category API + * @package GNUsocial + * @author Evan Prodromou + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later */ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction { @@ -46,20 +40,20 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction const MAXCOUNT = 200; const MAXNOTICES = 3200; - var $repeats = null; - var $cnt = self::DEFAULTCOUNT; - var $page = 1; - var $since_id = null; - var $max_id = null; + public $repeats = null; + public $cnt = self::DEFAULTCOUNT; + public $page = 1; + public $since_id = null; + public $max_id = null; /** * Take arguments for running * * @param array $args $_REQUEST args * - * @return boolean success flag + * @return bool success flag */ - function prepare(array $args = array()) + public function prepare(array $args = []) { parent::prepare($args); @@ -83,7 +77,7 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction * * @return void */ - function handle() + public function handle() { parent::handle(); @@ -101,7 +95,9 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction // TRANS: Subtitle of API time with retweets of me. // TRANS: %1$s is the StatusNet sitename, %2$s is the user nickname, %3$s is the user profile name. _('%1$s notices that %2$s / %3$s has repeated.'), - $sitename, $this->auth_user->nickname, $profile->getBestName() + $sitename, + $this->auth_user->nickname, + $profile->getBestName() ); $taguribase = TagURI::base(); @@ -109,18 +105,15 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction $link = common_local_url( 'all', - array('nickname' => $this->auth_user->nickname) + ['nickname' => $this->auth_user->nickname] ); - // This is a really bad query for some reason - - if (!common_config('performance', 'high')) { - $strm = $this->auth_user->repeatsOfMe($offset, $limit, $this->since_id, $this->max_id); - } else { - $strm = new Notice(); - $strm->whereAdd('0 = 1'); - $strm->find(); - } + $strm = $this->auth_user->repeatsOfMe( + $offset, + $limit, + $this->since_id, + $this->max_id + ); switch ($this->format) { case 'xml': @@ -165,7 +158,7 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction * * @return boolean is read only action? */ - function isReadOnly($args) + public function isReadOnly($args) { return true; } diff --git a/modules/Share/lib/repeatsofmenoticestream.php b/modules/Share/lib/repeatsofmenoticestream.php index 672e3dd973..e7af1ed8b5 100644 --- a/modules/Share/lib/repeatsofmenoticestream.php +++ b/modules/Share/lib/repeatsofmenoticestream.php @@ -1,53 +1,49 @@ . + /** - * StatusNet - the distributed open-source microblogging tool - * Copyright (C) 2011, StatusNet, Inc. - * * Stream of notices that are repeats of mine - * - * PHP version 5 - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . * * @category Stream - * @package StatusNet + * @package GNUsocial * @author Evan Prodromou * @copyright 2011 StatusNet, Inc. - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 - * @link http://status.net/ + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later */ -if (!defined('GNUSOCIAL')) { exit(1); } +defined('GNUSOCIAL') || die(); /** * Stream of notices that are repeats of mine * * @category Stream - * @package StatusNet + * @package GNUsocial * @author Evan Prodromou * @copyright 2011 StatusNet, Inc. - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 - * @link http://status.net/ + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later */ class RepeatsOfMeNoticeStream extends ScopingNoticeStream { - function __construct(Profile $target, Profile $scoped=null) + public function __construct(Profile $target, Profile $scoped=null) { - parent::__construct(new CachingNoticeStream(new RawRepeatsOfMeNoticeStream($target), - 'user:repeats_of_me:'.$target->getID()), - $scoped); + parent::__construct(new CachingNoticeStream( + new RawRepeatsOfMeNoticeStream($target), + 'user:repeats_of_me:' . $target->getID() + ), $scoped); } } @@ -55,57 +51,41 @@ class RepeatsOfMeNoticeStream extends ScopingNoticeStream * Raw stream of notices that are repeats of mine * * @category Stream - * @package StatusNet + * @package GNUsocial * @author Evan Prodromou * @copyright 2011 StatusNet, Inc. - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 - * @link http://status.net/ + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later */ class RawRepeatsOfMeNoticeStream extends NoticeStream { protected $target; - function __construct(Profile $target) + public function __construct(Profile $target) { $this->target = $target; } - function getNoticeIds($offset, $limit, $since_id, $max_id) + public function getNoticeIds($offset, $limit, $since_id, $max_id) { - $qry = - 'SELECT DISTINCT original.id AS id ' . - 'FROM notice original JOIN notice rept ON original.id = rept.repeat_of ' . - 'WHERE original.profile_id = ' . $this->target->getID() . ' '; - - $since = Notice::whereSinceId($since_id, 'original.id', 'original.created'); - if ($since) { - $qry .= "AND ($since) "; - } - - $max = Notice::whereMaxId($max_id, 'original.id', 'original.created'); - if ($max) { - $qry .= "AND ($max) "; - } - - $qry .= 'ORDER BY original.created, original.id DESC '; - - if (!is_null($offset)) { - $qry .= "LIMIT $limit OFFSET $offset"; - } - - $ids = array(); - $notice = new Notice(); - $notice->query($qry); + $notice->selectAdd(); + $notice->selectAdd('notice.id'); - while ($notice->fetch()) { - $ids[] = $notice->id; + $notice->joinAdd(['id', 'notice:repeat_of'], 'LEFT', 'repeat'); + $notice->whereAdd('repeat.repeat_of IS NOT NULL'); + $notice->whereAdd('notice.profile_id = ' . $this->target->getID()); + + Notice::addWhereSinceId($notice, $since_id); + Notice::addWhereMaxId($notice, $max_id); + + $notice->orderBy('notice.created DESC, notice.id DESC'); + $notice->limit($offset, $limit); + + if (!$notice->find()) { + return []; } - $notice->free(); - $notice = NULL; - - return $ids; + return $notice->fetchAll('id'); } } diff --git a/plugins/Embed/scripts/fixup_files.php b/plugins/Embed/scripts/fixup_files.php index cc7eb52e3b..18f22bc5e0 100755 --- a/plugins/Embed/scripts/fixup_files.php +++ b/plugins/Embed/scripts/fixup_files.php @@ -52,32 +52,32 @@ if (!($broken ^ $h_bug)) { die(); } -$query = " - SELECT DISTINCT - file_to_post.file_id - FROM - file_to_post - INNER JOIN - file ON file.id = file_to_post.file_id - INNER JOIN - notice ON notice.id = file_to_post.post_id - WHERE"; +$fn = new DB_DataObject(); + +$query = <<<'END' + SELECT file_to_post.file_id + FROM file_to_post + INNER JOIN file ON file_to_post.file_id = file.id + INNER JOIN notice ON file_to_post.post_id = notice.id + WHERE + END; -$f = new File(); if ($h_bug) { - $query .= " file.title = 'h' - AND file.mimetype = 'h' - AND file.size = 0 - AND file.protected = 0"; + $query .= <<<'END' + file.title = 'h' + AND file.mimetype = 'h' + AND file.size = 0 + AND file.protected = 0 + END; } elseif ($broken) { - $query .= " file.filename is NULL"; + $query .= ' file.filename IS NULL'; } -$query .= empty($limit) ? "" : " AND notice.modified >= '{$limit}' ORDER BY notice.modified ASC"; +if (!empty($limit)) { + $query .= " AND notice.modified >= '{$fn->escape($limit)}'"; +} +$query .= ' GROUP BY file_to_post.file_id ORDER BY MAX(notice.modified)'; -// echo $query; - -$fn = new DB_DataObject(); $fn->query($query); if ($h_bug) { @@ -133,14 +133,15 @@ while ($fn->fetch()) { echo " (ok, but embedding lookup failed)\n"; } } - } elseif ($broken && - (!$data instanceof File_embed || - empty($data->title) || - empty($f->title) - || - ($thumb instanceof File_thumbnail && empty($thumb->filename)) - )) { - + } elseif ( + $broken + && ( + !($data instanceof File_embed) + || empty($data->title) + || empty($f->title) + || ($thumb instanceof File_thumbnail && empty($thumb->filename)) + ) + ) { // print_r($thumb); if (!$dry) { diff --git a/plugins/StoreRemoteMedia/scripts/removeRemoteMedia.php b/plugins/StoreRemoteMedia/scripts/removeRemoteMedia.php index 4e7a0999d5..5961418784 100755 --- a/plugins/StoreRemoteMedia/scripts/removeRemoteMedia.php +++ b/plugins/StoreRemoteMedia/scripts/removeRemoteMedia.php @@ -59,25 +59,23 @@ if (empty($max_date)) { exit(1); } -$query = " - SELECT DISTINCT - file_to_post.file_id - FROM - file_to_post - INNER JOIN - file ON file.id = file_to_post.file_id - INNER JOIN - notice ON notice.id = file_to_post.post_id - WHERE - notice.is_local = 0 "; - -$query .= $image_only ? " AND file.width IS NOT NULL AND file.height IS NOT NULL " : ""; - -$query .= $include_previews ? "" : " AND file.filehash IS NOT NULL "; -$query .= " AND notice.modified <= '{$max_date}' ORDER BY notice.modified ASC"; - $fn = new DB_DataObject(); -$fn->query($query); +$fn->query(sprintf( + <<<'END' + SELECT file_to_post.file_id + FROM file_to_post + INNER JOIN file ON file_to_post.file_id = file.id + INNER JOIN notice ON file_to_post.post_id = notice.id + WHERE notice.is_local = 0 + %1$s%2$sAND notice.modified <= '%3$s' + GROUP BY file_to_post.file_id + ORDER BY MAX(notice.modified) + END, + $image_only ? 'AND file.width IS NOT NULL AND file.height IS NOT NULL ' : '', + $include_previews ? '' : 'AND file.filehash IS NOT NULL ', + $fn->escape($max_date) +)); + while ($fn->fetch()) { $file = File::getByID($fn->file_id); $file_info_id = $file->getID();