Merge pull request #103 from macbre/having_clause

Report queries with HAVING clause

Author: macbre

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 *`)
+  * reports queries that are not quite kosher (e.g. `LIKE "%foo%"`, `INSERT IGNORE`, `SELECT *`, `HAVING` clause)
 
 This tool **supports MySQL 5.5, 5.6, 5.7, 8.0 and MariaDB 10.0, 10.2** and runs under **Python 2.7, 3.4, 3.5 and 3.6**.
 
@@ -237,6 +237,13 @@ select_star → table affected: bar
 
   - query: SELECT t.* FROM bar AS t;
 
+------------------------------------------------------------
+having_clause → table affected: sales
+
+✗ "SELECT s.cust_id,count(s.cust_id) FROM SH.sales s ..." query uses HAVING clause
+
+  - query: SELECT s.cust_id,count(s.cust_id) FROM SH.sales s GROUP BY s.cust_id HAVING s.cust_id != '1660' AND s.cust_id != '2'
+
 (...)
 
 ------------------------------------------------------------
@@ -280,6 +287,8 @@ Outputs YML file with results and metadata.
 
 ## Checks
 
+> **Number of checks**: 17
+
 * `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)
 * `missing_primary_index`: reports tables with no primary or unique key (see [MySQL bug #76252](https://bugs.mysql.com/bug.php?id=76252) and [Wikia/app#9863](https://github.com/Wikia/app/pull/9863))
@@ -301,6 +310,7 @@ Outputs YML file with results and metadata.
 * `selects_with_like`: reports SELECT queries that use `LIKE '%foo'` conditions (they can not use an index)
 * `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)
 
 ## Success stories
 

indexdigest/cli/script.py

@@ -48,7 +48,8 @@
     check_insert_ignore_queries, \
     check_single_column, \
     check_empty_tables, \
-    check_select_star
+    check_select_star, \
+    check_having_clause
 
 
 def main():
@@ -101,6 +102,7 @@ def main():
             check_selects_with_like(database, queries=queries),
             check_insert_ignore_queries(database, queries=queries),
             check_select_star(database, queries=queries),
+            check_having_clause(database, queries=queries),
         )
 
     # handle --format

indexdigest/linters/__init__.py

@@ -17,3 +17,4 @@
 from .linter_0075_test_tables import check_test_tables
 from .linter_0089_empty_tables import check_empty_tables
 from .linter_0092_select_star import check_select_star
+from .linter_0093_having_clause import check_having_clause

indexdigest/linters/linter_0093_having_clause.py

@@ -0,0 +1,45 @@
+"""
+This linter checks for select queries with HAVING clause
+"""
+from sqlparse.tokens import Keyword
+from sql_metadata import preprocess_query, get_query_tables, get_query_tokens
+
+from indexdigest.utils import LinterEntry, shorten_query, is_select_query
+
+
+def query_has_having_clause(query):
+    """
+    Checks if provided query uses HAVING clause
+    :type query str
+    :rtype bool
+    """
+    if not is_select_query(query):
+        return False
+
+    query = preprocess_query(query)
+    tokens = get_query_tokens(query)
+
+    for token in tokens:
+        if token.ttype is Keyword and str(token).upper() == 'HAVING':
+            return True
+
+    return False
+
+
+def check_having_clause(_, queries):
+    """
+    :type queries list[str]
+    :rtype: list[LinterEntry]
+    """
+    queries_with_having_clause = [
+        query for query in queries
+        if query_has_having_clause(query)
+    ]
+
+    for query in queries_with_having_clause:
+        table_name = get_query_tables(query)[0]
+
+        yield LinterEntry(linter_type='having_clause', table_name=table_name,
+                          message='"{}" query uses HAVING clause'.
+                          format(shorten_query(query)),
+                          context={"query": query})

indexdigest/test/linters/test_0093_having_clause.py

@@ -0,0 +1,45 @@
+from __future__ import print_function
+
+from unittest import TestCase
+
+from indexdigest.linters.linter_0093_having_clause import query_has_having_clause, check_having_clause
+from indexdigest.test import DatabaseTestMixin, read_queries_from_log
+
+
+class TestLinter(TestCase, DatabaseTestMixin):
+
+    def test_query_has_having_clause(self):
+        assert query_has_having_clause('SELECT * FROM foo having bar = 2')
+        assert query_has_having_clause('SELECT * FROM foo HAVING bar = 2')
+
+        assert query_has_having_clause("SELECT * FROM 0019_queries_not_using_indices "
+                                       "WHERE foo = 'foo' HAVING bar = 'test'")
+        assert query_has_having_clause("SELECT s.cust_id,count(s.cust_id) FROM SH.sales s "
+                                       "GROUP BY s.cust_id HAVING s.cust_id != '1660' AND s.cust_id != '2'")
+
+        assert query_has_having_clause('SELECT * FROM foo') is False
+        assert query_has_having_clause('SELECT * FROM foo_having LIMIT 10') is False
+        assert query_has_having_clause('SELECT /* having */ id FROM foo') is False
+
+        assert query_has_having_clause('INSERT 42 INTO having') is False
+
+    def test_having_clause(self):
+        reports = list(check_having_clause(self.connection, read_queries_from_log('0093-having-clause-log')))
+
+        print(list(map(str, reports)))
+
+        assert len(reports) == 3
+
+        assert str(reports[0]) == 'foo: "SELECT * FROM foo HAVING bar = 2" query uses HAVING clause'
+        assert reports[0].table_name == 'foo'
+        assert reports[0].context['query'] == 'SELECT * FROM foo HAVING bar = 2;'
+
+        assert str(reports[1]) == 'sales: "SELECT s.cust_id,count(s.cust_id) ' \
+                                  'FROM SH.sales s ..." query uses HAVING clause'
+        assert reports[1].table_name == 'sales'
+
+        assert str(reports[2]) == '0019_queries_not_using_indices: "SELECT * FROM ' \
+                                  '`0019_queries_not_using_indices` WHE..." query uses HAVING clause'
+        assert reports[2].table_name == '0019_queries_not_using_indices'
+
+        # assert False

sql/0093-having-clause-log

@@ -0,0 +1,4 @@
+-- Rewriting the query's HAVING clause into a predicate will enable the use of indexes during query processing.
+SELECT * FROM foo HAVING bar = 2;
+SELECT s.cust_id,count(s.cust_id) FROM SH.sales s GROUP BY s.cust_id HAVING s.cust_id != '1660' AND s.cust_id != '2'
+SELECT * FROM `0019_queries_not_using_indices` WHERE foo = 'foo' HAVING bar = 'test';