[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.
This commit is contained in:
Alexei Sorokin 2020-08-27 11:15:39 +03:00 committed by Diogo Peralta Cordeiro
parent 300c4e3d04
commit d71eea1ba4
4 changed files with 130 additions and 158 deletions

View File

@ -1,44 +1,38 @@
<?php <?php
// This file is part of GNU social - https://www.gnu.org/software/social
//
// GNU social 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.
//
// GNU social 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 GNU social. If not, see <http://www.gnu.org/licenses/>.
/** /**
* StatusNet, the distributed open-source microblogging tool
*
* Show authenticating user's most recent notices that have been repeated * 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 <http://www.gnu.org/licenses/>.
*
* @category API * @category API
* @package StatusNet * @package GNUsocial
* @author Evan Prodromou <evan@status.net> * @author Evan Prodromou <evan@status.net>
* @copyright 2009 StatusNet, Inc. * @copyright 2009 StatusNet, Inc.
* @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
* @link http://status.net/
*/ */
if (!defined('STATUSNET')) { defined('GNUSOCIAL') || die();
exit(1);
}
/** /**
* Show authenticating user's most recent notices that have been repeated * Show authenticating user's most recent notices that have been repeated
* *
* @category API * @category API
* @package StatusNet * @package GNUsocial
* @author Evan Prodromou <evan@status.net> * @author Evan Prodromou <evan@status.net>
* @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
* @link http://status.net/
*/ */
class ApiTimelineRetweetsOfMeAction extends ApiAuthAction class ApiTimelineRetweetsOfMeAction extends ApiAuthAction
{ {
@ -46,20 +40,20 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction
const MAXCOUNT = 200; const MAXCOUNT = 200;
const MAXNOTICES = 3200; const MAXNOTICES = 3200;
var $repeats = null; public $repeats = null;
var $cnt = self::DEFAULTCOUNT; public $cnt = self::DEFAULTCOUNT;
var $page = 1; public $page = 1;
var $since_id = null; public $since_id = null;
var $max_id = null; public $max_id = null;
/** /**
* Take arguments for running * Take arguments for running
* *
* @param array $args $_REQUEST args * @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); parent::prepare($args);
@ -83,7 +77,7 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction
* *
* @return void * @return void
*/ */
function handle() public function handle()
{ {
parent::handle(); parent::handle();
@ -101,7 +95,9 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction
// TRANS: Subtitle of API time with retweets of me. // 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. // 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.'), _('%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(); $taguribase = TagURI::base();
@ -109,18 +105,15 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction
$link = common_local_url( $link = common_local_url(
'all', 'all',
array('nickname' => $this->auth_user->nickname) ['nickname' => $this->auth_user->nickname]
); );
// This is a really bad query for some reason $strm = $this->auth_user->repeatsOfMe(
$offset,
if (!common_config('performance', 'high')) { $limit,
$strm = $this->auth_user->repeatsOfMe($offset, $limit, $this->since_id, $this->max_id); $this->since_id,
} else { $this->max_id
$strm = new Notice(); );
$strm->whereAdd('0 = 1');
$strm->find();
}
switch ($this->format) { switch ($this->format) {
case 'xml': case 'xml':
@ -165,7 +158,7 @@ class ApiTimelineRetweetsOfMeAction extends ApiAuthAction
* *
* @return boolean is read only action? * @return boolean is read only action?
*/ */
function isReadOnly($args) public function isReadOnly($args)
{ {
return true; return true;
} }

View File

@ -1,53 +1,49 @@
<?php <?php
// This file is part of GNU social - https://www.gnu.org/software/social
//
// GNU social 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.
//
// GNU social 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 GNU social. If not, see <http://www.gnu.org/licenses/>.
/** /**
* StatusNet - the distributed open-source microblogging tool
* Copyright (C) 2011, StatusNet, Inc.
*
* Stream of notices that are repeats of mine * 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 <http://www.gnu.org/licenses/>.
*
* @category Stream * @category Stream
* @package StatusNet * @package GNUsocial
* @author Evan Prodromou <evan@status.net> * @author Evan Prodromou <evan@status.net>
* @copyright 2011 StatusNet, Inc. * @copyright 2011 StatusNet, Inc.
* @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
* @link http://status.net/
*/ */
if (!defined('GNUSOCIAL')) { exit(1); } defined('GNUSOCIAL') || die();
/** /**
* Stream of notices that are repeats of mine * Stream of notices that are repeats of mine
* *
* @category Stream * @category Stream
* @package StatusNet * @package GNUsocial
* @author Evan Prodromou <evan@status.net> * @author Evan Prodromou <evan@status.net>
* @copyright 2011 StatusNet, Inc. * @copyright 2011 StatusNet, Inc.
* @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
* @link http://status.net/
*/ */
class RepeatsOfMeNoticeStream extends ScopingNoticeStream 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), parent::__construct(new CachingNoticeStream(
'user:repeats_of_me:'.$target->getID()), new RawRepeatsOfMeNoticeStream($target),
$scoped); 'user:repeats_of_me:' . $target->getID()
), $scoped);
} }
} }
@ -55,57 +51,41 @@ class RepeatsOfMeNoticeStream extends ScopingNoticeStream
* Raw stream of notices that are repeats of mine * Raw stream of notices that are repeats of mine
* *
* @category Stream * @category Stream
* @package StatusNet * @package GNUsocial
* @author Evan Prodromou <evan@status.net> * @author Evan Prodromou <evan@status.net>
* @copyright 2011 StatusNet, Inc. * @copyright 2011 StatusNet, Inc.
* @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
* @link http://status.net/
*/ */
class RawRepeatsOfMeNoticeStream extends NoticeStream class RawRepeatsOfMeNoticeStream extends NoticeStream
{ {
protected $target; protected $target;
function __construct(Profile $target) public function __construct(Profile $target)
{ {
$this->target = $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 = new Notice();
$notice->query($qry); $notice->selectAdd();
$notice->selectAdd('notice.id');
while ($notice->fetch()) { $notice->joinAdd(['id', 'notice:repeat_of'], 'LEFT', 'repeat');
$ids[] = $notice->id; $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(); return $notice->fetchAll('id');
$notice = NULL;
return $ids;
} }
} }

View File

@ -52,32 +52,32 @@ if (!($broken ^ $h_bug)) {
die(); die();
} }
$query = " $fn = new DB_DataObject();
SELECT DISTINCT
file_to_post.file_id $query = <<<'END'
FROM SELECT file_to_post.file_id
file_to_post FROM file_to_post
INNER JOIN INNER JOIN file ON file_to_post.file_id = file.id
file ON file.id = file_to_post.file_id INNER JOIN notice ON file_to_post.post_id = notice.id
INNER JOIN WHERE
notice ON notice.id = file_to_post.post_id END;
WHERE";
$f = new File();
if ($h_bug) { if ($h_bug) {
$query .= " file.title = 'h' $query .= <<<'END'
AND file.mimetype = 'h' file.title = 'h'
AND file.size = 0 AND file.mimetype = 'h'
AND file.protected = 0"; AND file.size = 0
AND file.protected = 0
END;
} elseif ($broken) { } 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); $fn->query($query);
if ($h_bug) { if ($h_bug) {
@ -133,14 +133,15 @@ while ($fn->fetch()) {
echo " (ok, but embedding lookup failed)\n"; echo " (ok, but embedding lookup failed)\n";
} }
} }
} elseif ($broken && } elseif (
(!$data instanceof File_embed || $broken
empty($data->title) || && (
empty($f->title) !($data instanceof File_embed)
|| || empty($data->title)
($thumb instanceof File_thumbnail && empty($thumb->filename)) || empty($f->title)
)) { || ($thumb instanceof File_thumbnail && empty($thumb->filename))
)
) {
// print_r($thumb); // print_r($thumb);
if (!$dry) { if (!$dry) {

View File

@ -59,25 +59,23 @@ if (empty($max_date)) {
exit(1); 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 = 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()) { while ($fn->fetch()) {
$file = File::getByID($fn->file_id); $file = File::getByID($fn->file_id);
$file_info_id = $file->getID(); $file_info_id = $file->getID();