Skip to content

Commit

Permalink
Do not use SELECT DISTINCT where possible
Browse files Browse the repository at this point in the history 
SELECT DISTINCT is bad for performance because it forces the use of
temporary tables [1]. Stop using it. As far as we could tell in our
testing, a simple s/SELECT DISTINCT/SELECT/ is enough in most places,
except two.

First, when listing a reader's projects. Readers are associated with
sections, not projects, and there's a many-to-one relation between
sections and projects, so we need the SELECT DISTINCT to avoid
repeating the project once for every section for a particular reader.

Second, when listing users according to a specific role. There's a
one-to-many relation between users and roles, and the latter are
stored as rows in the database. We need the SELECT DISTINCT to avoid
listing a user once for every role that they have.

Other than that, we seem to be able to get away with just not doing
DISTINCT.

[1] https://dba.stackexchange.com/questions/34674/mysql-performance-tuning-queries-stuck-on-copying-to-tmp-tables

Resolves: LibriVox#82
  • Loading branch information
@notartom
notartom committed Aug 14, 2021
1 parent f3bc85f commit 1ca40d8
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 31 deletions.
8 changes: 4 additions & 4 deletions application/libraries/Librivox_search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function advanced_title_search($params)

$sql .= '
SELECT DISTINCT "title" AS blade, ' . implode(',' , $cols) . '
SELECT "title" AS blade, ' . implode(',' , $cols) . '
FROM search_table st
Expand Down Expand Up @@ -227,7 +227,7 @@ function advanced_title_search($params)
$sql .= '
UNION
SELECT DISTINCT "title" AS blade, ' . implode(',' , $cols) . '
SELECT "title" AS blade, ' . implode(',' , $cols) . '
FROM search_table st
Expand Down Expand Up @@ -272,7 +272,7 @@ function advanced_title_search($params)
$sql .= '
UNION
SELECT DISTINCT "group" AS blade,' . implode(',' , $cols) . '
SELECT "group" AS blade,' . implode(',' , $cols) . '
FROM search_table st
Expand Down Expand Up @@ -452,4 +452,4 @@ private function _get_projects_by_reader($reader, $exact_match)
}


}
}
6 changes: 3 additions & 3 deletions application/libraries/Librivox_simple_search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function simple_search($params)
// q is in the title, and not in a group. Joins language & author for fields.

$sql .= '
SELECT DISTINCT "title" AS blade, ' . implode(',' , $cols) . '
SELECT "title" AS blade, ' . implode(',' , $cols) . '
FROM search_table st
Expand Down Expand Up @@ -114,7 +114,7 @@ function simple_search($params)
UNION
SELECT DISTINCT "group" AS blade,' . implode(',' , $cols) . '
SELECT "group" AS blade,' . implode(',' , $cols) . '
FROM search_table st
Expand Down Expand Up @@ -337,4 +337,4 @@ function simple_search($params)

}

}
}
18 changes: 9 additions & 9 deletions application/models/Project_model.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function get_projects($params)
OR p.person_mc_id = '.$user_projects.'
OR p.person_pl_id = '.$user_projects.'
OR reader_data.reader_id = '.$user_projects.')';
}
}

$sql .= ' ORDER BY p.title '. $limit;

Expand All @@ -70,7 +70,7 @@ function get_projects($params)

function get_lastest_releases($limit=10)
{
$sql = 'SELECT DISTINCT p.id, p.title_prefix, p.title, p.url_librivox, p.description, p.project_type, p.coverart_thumbnail, p.date_catalog, COALESCE(l.two_letter_code, l.three_letter_code) AS language_code, language
$sql = 'SELECT p.id, p.title_prefix, p.title, p.url_librivox, p.description, p.project_type, p.coverart_thumbnail, p.date_catalog, COALESCE(l.two_letter_code, l.three_letter_code) AS language_code, language
FROM projects p
JOIN languages l ON (l.id = p.language_id)
WHERE p.status = "complete"
Expand Down Expand Up @@ -154,7 +154,7 @@ function get_project_authors($project_id)

function get_project_readers($project_id)
{
$sql = 'SELECT DISTINCT u.id AS reader_id, u.display_name
$sql = 'SELECT u.id AS reader_id, u.display_name
FROM users u
JOIN section_readers sr ON (sr.reader_id = u.id)
JOIN sections s ON (s.id = sr.section_id)
Expand All @@ -168,7 +168,7 @@ function get_project_readers($project_id)

function create_project_reader_list($project_id)
{
$sql = 'SELECT DISTINCT u.display_name
$sql = 'SELECT u.display_name
FROM users u
JOIN section_readers sr ON (sr.reader_id = u.id)
JOIN sections s ON (s.id = sr.section_id)
Expand Down Expand Up @@ -221,7 +221,7 @@ function get_projects_by_author($params)


$sql = '
SELECT DISTINCT "project" AS primary_type, p.id AS primary_key, p.id, p.project_type, p.title_prefix, p.title,
SELECT "project" AS primary_type, p.id AS primary_key, p.id, p.project_type, p.title_prefix, p.title,
COALESCE(p.date_catalog, "2001-01-01" ), p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size, l.language , p.url_forum
FROM projects p
JOIN project_authors pa ON (pa.project_id = p.id)
Expand Down Expand Up @@ -253,7 +253,7 @@ function get_projects_by_author($params)

$sql .= '
UNION
SELECT DISTINCT "section" AS primary_type, s.id AS primary_key, p.id, p.project_type, "", CONCAT(s.title, " (in ", COALESCE(p.title_prefix, "" ), " ", p.title, ")") as title,
SELECT "section" AS primary_type, s.id AS primary_key, p.id, p.project_type, "", CONCAT(s.title, " (in ", COALESCE(p.title_prefix, "" ), " ", p.title, ")") as title,
COALESCE(p.date_catalog, "2001-01-01" ), p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size, l.language , p.url_forum
FROM projects p
JOIN sections s ON (p.id = s.project_id)
Expand Down Expand Up @@ -318,7 +318,7 @@ function get_projects_for_title_menu($params)
}

$sql = '
SELECT DISTINCT "project" AS primary_type, p.id AS primary_key, p.id, p.project_type, p.title_prefix, p.title,
SELECT "project" AS primary_type, p.id AS primary_key, p.id, p.project_type, p.title_prefix, p.title,
COALESCE(p.date_catalog, "2001-01-01" ), p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size, l.language, p.url_forum
FROM projects p
Expand Down Expand Up @@ -351,7 +351,7 @@ function get_projects_for_title_menu($params)

function get_projects_by_reader($params)
{
$sql = 'SELECT DISTINCT p.id, p.project_type, p.title_prefix, p.title, p.date_catalog, p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size,
$sql = 'SELECT p.id, p.project_type, p.title_prefix, p.title, p.date_catalog, p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size,
l.two_letter_code, l.language , p.url_forum
FROM projects p
JOIN project_readers pr ON (pr.project_id = p.id)
Expand Down Expand Up @@ -388,7 +388,7 @@ function get_projects_by_reader($params)
function get_projects_by_group($params)
{

$sql = 'SELECT DISTINCT p.id, p.project_type, p.title_prefix, p.title, p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size,
$sql = 'SELECT p.id, p.project_type, p.title_prefix, p.title, p.url_librivox, p.status, p.coverart_thumbnail, p.zip_url, p.zip_size,
l.two_letter_code, l.language, p.url_forum
FROM projects p
JOIN group_projects gp ON (gp.project_id = p.id)
Expand Down
4 changes: 2 additions & 2 deletions application/models/Project_reader_model.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Project_reader_model extends MY_Model {

function get_all_distinct($offset=0, $limit=CATALOG_RESULT_COUNT){

$sql = "SELECT DISTINCT reader_id, display_name, username, COALESCE(NULLIF(display_name,''), username)
$sql = "SELECT reader_id, display_name, username, COALESCE(NULLIF(display_name,''), username)
FROM project_readers
ORDER BY 4 ASC ";

Expand All @@ -15,4 +15,4 @@ function get_all_distinct($offset=0, $limit=CATALOG_RESULT_COUNT){

}

}
}
28 changes: 15 additions & 13 deletions application/models/User_model.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,30 @@ function get_users($params=array())
//$limit = 'LIMIT 0, 500';
$limit = '';

$sql = 'SELECT DISTINCT u.*
FROM ' . $this->_table . ' u
LEFT OUTER JOIN users_groups ug ON (ug.user_id = u.id)
LEFT OUTER JOIN roles r ON (ug.group_id = r.id)
WHERE u.id NOT IN (1,2) AND u.active = 1 '; //set these to constants
if (!empty($params['user_type']) and $params['user_type'] != 'all')
{
$sql = 'SELECT DISTINCT u.id, u.username, u.display_name, u.email, u.website
FROM ' . $this->_table . ' u
LEFT OUTER JOIN users_groups ug ON (ug.user_id = u.id)
LEFT OUTER JOIN roles r ON (ug.group_id = r.id)
WHERE u.id NOT IN (1,2) AND u.active = 1
AND r.name = '. $this->db->escape($params['user_type']) .' ';
} else {
$sql = 'SELECT u.id, u.username, u.display_name, u.email, u.website
FROM ' . $this->_table . ' u
WHERE u.id NOT IN (1,2) AND u.active = 1 '; //set these to constants
}

if (!empty($params['user_search']))
{
$user_like = $this->db->escape('%' . $params['user_search'] . '%');
$sql .= ' AND (u.username LIKE ' . $user_like . ' OR u.email LIKE ' . $user_like . ') ' ;
}

if (!empty($params['user_type']))
{
if ($params['user_type'] == 'all')
$limit = '';
else
$sql .= ' AND r.name = '. $this->db->escape($params['user_type']) .' ';
}

$sql .= ' ORDER BY u.username '. $limit;

log_message('error', $sql);

$query = $this->db->query($sql);
return $query->result();
}
Expand Down

0 comments on commit 1ca40d8

Please sign in to comment.