Merge pull request #126 from macbre/high-offset-selects

Avoid high offsets in queries

Author: macbre

.travis.yml

@@ -23,7 +23,7 @@ matrix:
     - python: 3.6
       env: DOCKER_IMAGE=mysql:5.6
     - python: 3.6
-      env: DOCKER_IMAGE=mysql:8.0
+      env: DOCKER_IMAGE=mysql:8.0.3
     # @see https://hub.docker.com/_/mariadb/
     - python: 3.6
       env: DOCKER_IMAGE=mariadb:10.0  # used by Wikipedia
@@ -35,7 +35,7 @@ before_script:
   # @see https://hub.docker.com/_/mysql/
   - sudo service mysql stop  # stop any running MySQL server
   - sudo docker run -e MYSQL_ALLOW_EMPTY_PASSWORD=yes -d -p 3306:3306 $DOCKER_IMAGE
-  - sleep 15 # wait for MySQL server to be fully set up
+  - ./wait_for_mysql.sh
   - sudo docker ps
   # set up a database
   - mysql --protocol=tcp -u root -e "CREATE DATABASE index_digest; CREATE USER 'index_digest'@'%' IDENTIFIED BY 'qwerty'; GRANT ALL ON index_digest.* TO 'index_digest'@'%';"

README.md

@@ -13,7 +13,7 @@ Analyses your database queries and schema and suggests indices improvements. You
   * reports text columns with character set different than `utf`
   * reports queries that do not use indices
   * reports queries that use filesort, temporary file or full table scan
-  * reports queries that are not quite kosher (e.g. `LIKE "%foo%"`, `INSERT IGNORE`, `SELECT *`, `HAVING` clause)
+  * reports queries that are not quite kosher (e.g. `LIKE "%foo%"`, `INSERT IGNORE`, `SELECT *`, `HAVING` clause, high `OFFSET` in pagination queries)
 * if run with `--analyze-data` switch it:
   * reports tables with old data (by querying for `MIN()` value of time column) where data retency can be reviewed
   * reports tables with not up-to-date data (by querying for `MAX()` value of time column)
@@ -314,6 +314,15 @@ data_not_updated_recently → table affected: 0028_data_not_updated_recently
   - rows: 3
   - table_size_mb: 0.015625
 
+------------------------------------------------------------
+high_offset_selects → table affected: page
+
+✗ "SELECT /* CategoryPaginationViewer::processSection..." query uses too high offset impacting the performance
+
+  - query: SELECT /* CategoryPaginationViewer::processSection */  page_namespace,page_title,page_len,page_is_redirect,cl_sortkey_prefix  FROM `page` INNER JOIN `categorylinks` FORCE INDEX (cl_sortkey) ON ((cl_from = page_id))  WHERE cl_type = 'page' AND cl_to = 'Spotify/Song'  ORDER BY cl_sortkey LIMIT 927600,200
+  - limit: 200
+  - offset: 927600
+
 ------------------------------------------------------------
 Queries performed: 100
 ```
@@ -358,7 +367,7 @@ Outputs YML file with results and metadata.
 
 You can select which checks should be reported by the tool by using `--checks` command line option. Certain checks can also be skipped via `--skip-checks` option. Refer to `index_digest --help` for examples.
 
-> **Number of checks**: 20
+> **Number of checks**: 21
 
 * `redundant_indices`: reports indices that are redundant and covered by other
 * `non_utf_columns`: reports text columns that have characters encoding set to `latin1` (utf is the way to go)
@@ -383,6 +392,7 @@ You can select which checks should be reported by the tool by using `--checks` c
 * `insert_ignore`: reports [queries using `INSERT IGNORE`](https://medium.com/legacy-systems-diary/things-to-avoid-episode-1-insert-ignore-535b4c24406b)
 * `select_star`: reports [queries using `SELECT *`](https://github.com/jarulraj/sqlcheck/blob/master/docs/query/3001.md)
 * `having_clause`: reports [queries using `HAVING` clause](https://github.com/jarulraj/sqlcheck/blob/master/docs/query/3012.md)
+* `high_offset_selects`: report [SELECT queries using high OFFSET](https://www.percona.com/blog/2008/09/24/four-ways-to-optimize-paginated-displays/)
 
 ### Additional checks performed on tables data
 

indexdigest/cli/script.py

@@ -61,7 +61,8 @@
     check_having_clause, \
     check_data_too_old, \
     check_data_not_updated_recently, \
-    check_generic_primary_key
+    check_generic_primary_key, \
+    check_high_offset_selects
 
 
 def get_reports(database, sql_log=None, analyze_data=False):
@@ -110,6 +111,7 @@ def get_reports(database, sql_log=None, analyze_data=False):
             check_insert_ignore_queries(database, queries=queries),
             check_select_star(database, queries=queries),
             check_having_clause(database, queries=queries),
+            check_high_offset_selects(database, queries=queries),
         )
 
     # checks that require --analyze-data switch to be on (see #28)

indexdigest/linters/__init__.py

@@ -21,3 +21,4 @@
 from .linter_0092_select_star import check_select_star
 from .linter_0093_having_clause import check_having_clause
 from .linter_0094_generic_primary_key import check_generic_primary_key
+from .linter_0118_high_offset_selects import check_high_offset_selects

indexdigest/linters/linter_0118_high_offset_selects.py

@@ -0,0 +1,41 @@
+"""
+This linter checks for too high offset SELECT queries
+"""
+from collections import OrderedDict
+
+from sql_metadata import get_query_limit_and_offset, get_query_tables
+
+from indexdigest.utils import LinterEntry, shorten_query
+
+
+OFFSET_THRESHOLD = 1000
+
+
+def check_high_offset_selects(_, queries):
+    """
+    :type _ indexdigest.database.Database
+    :type queries list[str]
+    :rtype: list[LinterEntry]
+    """
+    for query in queries:
+        res = get_query_limit_and_offset(query)
+
+        if res is None:
+            continue
+
+        (limit, offset) = res
+
+        if offset < OFFSET_THRESHOLD:
+            continue
+
+        table_name = get_query_tables(query)[0]
+
+        context = OrderedDict()
+        context['query'] = query
+        context['limit'] = limit
+        context['offset'] = offset
+
+        yield LinterEntry(linter_type='high_offset_selects', table_name=table_name,
+                          message='"{}" query uses too high offset impacting the performance'.
+                          format(shorten_query(query)),
+                          context=context)

indexdigest/test/linters/test_0118_high_offset_selects.py

@@ -0,0 +1,25 @@
+from __future__ import print_function
+
+from unittest import TestCase
+
+from indexdigest.linters.linter_0118_high_offset_selects import check_high_offset_selects
+from indexdigest.test import DatabaseTestMixin, read_queries_from_log
+
+
+class TestLinter(TestCase, DatabaseTestMixin):
+
+    def test_high_offset_selects(self):
+        reports = list(check_high_offset_selects(
+            self.connection, queries=read_queries_from_log('0118-high-offset-selects-log')))
+
+        print(reports, reports[0].context)
+
+        self.assertEqual(len(reports), 1)
+
+        self.assertEqual(str(reports[0]), 'page: "SELECT /* CategoryPaginationViewer::processSection..." query uses too high offset impacting the performance')
+        self.assertEqual(reports[0].table_name, 'page')
+        self.assertEqual(str(reports[0].context['query']), "SELECT /* CategoryPaginationViewer::processSection */  page_namespace,page_title,page_len,page_is_redirect,cl_sortkey_prefix  FROM `page` INNER JOIN `categorylinks` FORCE INDEX (cl_sortkey) ON ((cl_from = page_id))  WHERE cl_type = 'page' AND cl_to = 'Spotify/Song'  ORDER BY cl_sortkey LIMIT 927600,200")
+        self.assertEqual(reports[0].context['limit'], 200)
+        self.assertEqual(reports[0].context['offset'], 927600)
+
+        # assert False

sql/0118-high-offset-selects-log

@@ -0,0 +1,7 @@
+-- no offset queries
+SELECT foo_limit FROM bar_offset
+-- not high enough query
+SELECT foo_limit FROM bar_offset LIMIT 50 OFFSET 100
+select * from 0020_big_table order by id limit 50, 5;
+-- offset queries
+SELECT /* CategoryPaginationViewer::processSection */  page_namespace,page_title,page_len,page_is_redirect,cl_sortkey_prefix  FROM `page` INNER JOIN `categorylinks` FORCE INDEX (cl_sortkey) ON ((cl_from = page_id))  WHERE cl_type = 'page' AND cl_to = 'Spotify/Song'  ORDER BY cl_sortkey LIMIT 927600,200

wait_for_mysql.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+# wait for MySQL server to be fully set up
+echo -n 'Waiting for MySQL server '
+
+until mysql --silent --protocol=tcp -uroot -e 'select 1' 1>/dev/null 2>/dev/null
+do
+	echo -n '.'
+	sleep 1
+done
+
+echo ' done'