Repository: kayak/pypika
Pull request #65 (2017-07-25 08:30:42)

Fixed union SQL for MySQL to no longer wrap queries in parentheses
Added a dialect option to not wrap queries in parentheses in a union and applied to mysql


Patch:
diff --git a/pypika/__init__.py b/pypika/__init__.py
index 52b1e94..e6d8485 100644
--- a/pypika/__init__.py
+++ b/pypika/__init__.py
@@ -55,4 +55,4 @@ from .utils import (JoinException,
 
 __author__ = "Timothy Heys"
 __email__ = "theys@kayak.com"
-__version__ = "0.6.3"
+__version__ = "0.6.1"
diff --git a/pypika/queries.py b/pypika/queries.py
index 2871dd3..bb24ab8 100644
--- a/pypika/queries.py
+++ b/pypika/queries.py
@@ -1,20 +1,26 @@
 # coding: utf8
 
-from pypika.enums import (Dialects,
-                          JoinType,
-                          UnionType)
-from pypika.utils import (JoinException,
-                          UnionException,
-                          RollupException,
-                          builder,
-                          alias_sql)
-from .terms import (Field,
-                    Star,
-                    Term,
-                    Function,
-                    ArithmeticExpression,
-                    Rollup,
-                    Tuple)
+from pypika.enums import (
+    Dialects,
+    JoinType,
+    UnionType,
+)
+from pypika.utils import (
+    JoinException,
+    RollupException,
+    UnionException,
+    alias_sql,
+    builder,
+)
+from .terms import (
+    ArithmeticExpression,
+    Field,
+    Function,
+    Rollup,
+    Star,
+    Term,
+    Tuple,
+)
 
 __author__ = "Timothy Heys"
 __email__ = "theys@kayak.com"
@@ -101,8 +107,10 @@ class Query(object):
 
     This class is immutable.
     """
-    QUOTE_CHARACTER = '"'
-    DIALECT = None
+
+    @classmethod
+    def _builder(cls):
+        return QueryBuilder()
 
     @classmethod
     def from_(cls, table):
@@ -117,7 +125,7 @@ class Query(object):
 
         :returns QueryBuilder
         """
-        return QueryBuilder(quote_char=cls.QUOTE_CHARACTER, dialect=cls.DIALECT).from_(table)
+        return cls._builder().from_(table)
 
     @classmethod
     def into(cls, table):
@@ -132,7 +140,7 @@ class Query(object):
 
         :returns QueryBuilder
         """
-        return QueryBuilder(quote_char=cls.QUOTE_CHARACTER, dialect=cls.DIALECT).into(table)
+        return cls._builder().into(table)
 
     @classmethod
     def select(cls, *terms):
@@ -148,47 +156,57 @@ class Query(object):
 
         :returns QueryBuilder
         """
-        return QueryBuilder(quote_char=cls.QUOTE_CHARACTER, dialect=cls.DIALECT).select(*terms)
+        return cls._builder().select(*terms)
 
 
 class MySQLQuery(Query):
     """
     Defines a query class for use with MySQL.
     """
-    QUOTE_CHARACTER = "`"
-    DIALECT = Dialects.MYSQL
+
+    @classmethod
+    def _builder(cls):
+        return QueryBuilder(quote_char='`', dialect=Dialects.MYSQL, wrap_union_queries=False)
 
 
 class VerticaQuery(Query):
     """
     Defines a query class for use with Vertica.
     """
-    QUOTE_CHARACTER = '"'
-    DIALECT = Dialects.VERTICA
+
+    @classmethod
+    def _builder(cls):
+        return QueryBuilder(dialect=Dialects.VERTICA)
 
 
 class OracleQuery(Query):
     """
     Defines a query class for use with Oracle.
     """
-    QUOTE_CHARACTER = '"'
-    DIALECT = Dialects.ORACLE
+
+    @classmethod
+    def _builder(cls):
+        return QueryBuilder(dialect=Dialects.ORACLE)
 
 
 class PostgreSQLQuery(Query):
     """
     Defines a query class for use with PostgreSQL.
     """
-    QUOTE_CHARACTER = '"'
-    DIALECT = Dialects.POSTGRESQL
+
+    @classmethod
+    def _builder(cls):
+        return QueryBuilder(dialect=Dialects.POSTGRESQL)
 
 
 class MSSQLQuery(Query):
     """
     Defines a query class for use with Microsoft SQL Server.
     """
-    QUOTE_CHARACTER = '"'
-    DIALECT = Dialects.MSSQL
+
+    @classmethod
+    def _builder(cls):
+        return QueryBuilder(dialect=Dialects.MSSQL)
 
 
 class QueryBuilder(Selectable, Term):
@@ -197,7 +215,7 @@ class QueryBuilder(Selectable, Term):
     state to be branched immutably.
     """
 
-    def __init__(self, quote_char='"', dialect=None):
+    def __init__(self, quote_char='"', dialect=None, wrap_union_queries=True):
         super(QueryBuilder, self).__init__(None)
 
         self._from = []
@@ -225,8 +243,10 @@ class QueryBuilder(Selectable, Term):
         self._select_into = False
 
         self._subquery_count = 0
+
         self.quote_char = quote_char
         self.dialect = dialect
+        self.wrap_union_queries = wrap_union_queries
 
     @builder
     def from_(self, selectable):
@@ -539,12 +559,12 @@ class QueryBuilder(Selectable, Term):
         if self._orderbys:
             querystring += self._orderby_sql(**kwargs)
 
-        if self._limit:
-            querystring += self._limit_sql()
-
         if self._offset:
             querystring += self._offset_sql()
 
+        if self._limit:
+            querystring += self._limit_sql()
+
         if subquery:
             querystring = '({query})'.format(query=querystring)
 
@@ -627,19 +647,21 @@ class QueryBuilder(Selectable, Term):
         )
 
     def _union_sql(self, querystring, quote_char=None, **kwargs):
-        if self._unions:
-            # Some queries require brackets for unions so easier to just always use them
-            querystring = "({})".format(querystring)
-
-            for (union_type, other) in self._unions:
-                other_querystring = other.get_sql(quote_char=quote_char)
-                if len(self._selects) != len(other._selects):
-                    raise UnionException("Queries must have an equal number of select statements in a union."
-                                         "\n\nMain Query:\n{query1}\n\nUnion Query:\n{query2}" \
-                                         .format(query1=querystring, query2=other_querystring))
-
-                querystring += ' UNION{type} ({query})' \
-                    .format(type=union_type.value, query=other_querystring)
+        if not self._unions:
+            return querystring
+
+        template = '({query}) UNION{type} ({union})' \
+            if self.wrap_union_queries \
+            else '{query} UNION{type} {union}'
+
+        for (union_type, other) in self._unions:
+            other_querystring = other.get_sql(quote_char=quote_char)
+            if len(self._selects) != len(other._selects):
+                raise UnionException("Queries must have an equal number of select statements in a union."
+                                     "\n\nMain Query:\n{query1}\n\nUnion Query:\n{query2}"
+                                     .format(query1=querystring, query2=other_querystring))
+
+            querystring = template.format(query=querystring, type=union_type.value, union=other_querystring)
 
         return querystring
 
diff --git a/pypika/terms.py b/pypika/terms.py
index 477b947..1cea8cd 100644
--- a/pypika/terms.py
+++ b/pypika/terms.py
@@ -1,63 +1,23 @@
 # coding: utf8
-import copy
 import re
 from datetime import date
-from functools import partial
 
 from aenum import Enum
 
-from pypika.enums import (
-    Boolean,
-    Dialects,
-    Equality,
-    Arithmetic,
-    Matching,
-)
-from pypika.utils import (
-    CaseException,
-    builder,
-    resolve_is_aggregate,
-    alias_sql,
-)
-import inspect
+from pypika.enums import (Boolean,
+                          Dialects,
+                          Equality,
+                          Arithmetic,
+                          Matching)
+from pypika.utils import (CaseException,
+                          builder,
+                          resolve_is_aggregate,
+                          alias_sql)
 
 __author__ = "Timothy Heys"
 __email__ = "theys@kayak.com"
 
 
-class Not(object):
-    def __init__(self, term):
-        self.term = term
-
-    def fields(self):
-        return self.term.fields() if self.term.fields else []
-
-    def get_sql(self, **kwargs):
-        return "NOT {term}".format(term=self.term.get_sql(**kwargs))
-
-    def __str__(self):
-        return self.get_sql(quote_char='"')
-
-    def __getattr__(self, item):
-        """
-        Delegate method calls to the class wrapped by Not().
-        Re-wrap methods on child classes of Term (e.g. isin, eg...) to retain 'NOT <term>' output.
-        """
-
-        item_func = getattr(self.term, item)
-
-        if not inspect.ismethod(item_func):
-            return item_func
-
-        def inner(inner_self, *args, **kwargs):
-            result = item_func(inner_self, *args, **kwargs)
-            if isinstance(result, (Term, )):
-                return Not(result)
-            return result
-
-        return inner
-
-
 class Term(object):
     is_aggregate = False
 
@@ -144,9 +104,6 @@ class Term(object):
     def bin_regex(self, pattern):
         return BasicCriterion(Matching.bin_regex, self, self._wrap(pattern))
 
-    def negate(self):
-        return Not(self)
-
     def __add__(self, other):
         return ArithmeticExpression(Arithmetic.add, self, self._wrap(other))
 
@@ -326,6 +283,10 @@ class Tuple(Term):
 
 
 class Criterion(Term):
+    def __init__(self, alias=None):
+        super(Criterion, self).__init__(alias)
+        self._is_negated = False
+
     def __and__(self, other):
         return ComplexCriterion(Boolean.and_, self, other)
 
@@ -335,6 +296,10 @@ class Criterion(Term):
     def __xor__(self, other):
         return ComplexCriterion(Boolean.xor_, self, other)
 
+    @builder
+    def negate(self):
+        self._is_negated = True
+
     def fields(self):
         raise NotImplementedError()
 
@@ -401,7 +366,6 @@ class ContainsCriterion(Criterion):
         super(ContainsCriterion, self).__init__(alias)
         self.term = term
         self.container = container
-        self._is_negated = False
 
     def fields(self):
         return self.term.fields() if self.term.fields else []
@@ -414,10 +378,6 @@ class ContainsCriterion(Criterion):
             not_='NOT ' if self._is_negated else ''
         )
 
-    def negate(self):
-        self._is_negated = True
-        return self
-
 
 class BetweenCriterion(Criterion):
     def __init__(self, term, start, end, alias=None):
@@ -460,8 +420,9 @@ class NullCriterion(Criterion):
         self.term = self.term.for_(table)
 
     def get_sql(self, **kwargs):
-        return "{term} IS NULL".format(
+        return "{term} IS{not_} NULL".format(
             term=self.term.get_sql(**kwargs),
+            not_=' NOT' if self._is_negated else ''
         )
 
     def fields(self):
@@ -639,7 +600,7 @@ class Function(Term):
             A copy of the field with it's table value replaced.
         """
         self.args = [param.for_(table) if hasattr(param, 'for_')
-                     else param
+                       else param
                      for param in self.args]
 
     def get_special_params_sql(self, **kwargs):
@@ -651,8 +612,8 @@ class Function(Term):
         return '{name}({args}{special})'.format(
             name=self.name,
             args=','.join(p.get_sql(with_alias=False, **kwargs)
-                          if hasattr(p, 'get_sql')
-                          else str(p)
+                            if hasattr(p, 'get_sql')
+                            else str(p)
                           for p in self.args),
             special=(' ' + special_params_sql) if special_params_sql else '',
         )


Test patch:
diff --git a/pypika/tests/test_criterions.py b/pypika/tests/test_criterions.py
index 870f11d..bfc3cf8 100644
--- a/pypika/tests/test_criterions.py
+++ b/pypika/tests/test_criterions.py
@@ -80,6 +80,13 @@ class CriterionTests(unittest.TestCase):
         self.assertEqual('"foo" IS NULL', str(c1))
         self.assertEqual('"crit"."foo" IS NULL', str(c2))
 
+    def test__criterion_is_not_null(self):
+        c1 = Field('foo').notnull()
+        c2 = Field('foo', table=self.t).notnull()
+
+        self.assertEqual('"foo" IS NOT NULL', str(c1))
+        self.assertEqual('"crit"."foo" IS NOT NULL', str(c2))
+
     def test__criterion_ne_number(self):
         c1 = Field('foo') != 1
         c2 = Field('foo', table=self.t).ne(0)
@@ -238,42 +245,6 @@ class CriterionTests(unittest.TestCase):
         self.assertEqual('"crit"."foo">=-1', str(c2))
 
 
-class NotTests(unittest.TestCase):
-    table_abc = Table('abc', alias='cx0')
-
-    def test_negate(self):
-        c1 = Field('foo') >= 1
-        c2 = c1.negate()
-
-        self.assertEqual('"foo">=1', str(c1))
-        self.assertEqual('NOT "foo">=1', str(c2))
-
-    def test_variable_access(self):
-        c1 = Field('foo').negate()
-
-        self.assertEqual(c1.is_aggregate, False)
-
-    def test_chained_function(self):
-        field1 = Field('foo').negate()
-        field2 = field1.eq('bar')
-
-        self.assertEqual('NOT "foo"', str(field1))
-        self.assertEqual('NOT "foo"=\'bar\'', str(field2))
-        self.assertIsNot(field1, field2)
-
-    def test_not_null(self):
-        c1 = Field('foo').notnull()
-        c2 = Field('foo', table=self.table_abc).notnull()
-
-        self.assertEqual('NOT "foo" IS NULL', str(c1))
-        self.assertEqual('NOT "cx0"."foo" IS NULL', str(c2))
-
-    def test_notnullcriterion_for_table(self):
-        f = self.table_abc.foo.notnull().for_(self.table_abc)
-
-        self.assertEqual('NOT "cx0"."foo" IS NULL', str(f))
-
-
 class BetweenTests(unittest.TestCase):
     t = Table('abc', alias='btw')
 
@@ -488,3 +459,8 @@ class CriterionOperationsTests(unittest.TestCase):
         f = self.table_abc.foo.isnull().for_(self.table_efg)
 
         self.assertEqual('"cx1"."foo" IS NULL', str(f))
+
+    def test_notnullcriterion_for_table(self):
+        f = self.table_abc.foo.notnull().for_(self.table_efg)
+
+        self.assertEqual('"cx1"."foo" IS NOT NULL', str(f))
diff --git a/pypika/tests/test_joins.py b/pypika/tests/test_joins.py
index 405e8f2..51d6e04 100644
--- a/pypika/tests/test_joins.py
+++ b/pypika/tests/test_joins.py
@@ -1,7 +1,17 @@
 # coding: utf8
 import unittest
 
-from pypika import Query, Table, Tables, JoinException, functions as fn, JoinType, UnionException, Interval
+from pypika import (
+    Query,
+    Table,
+    Tables,
+    JoinException,
+    functions as fn,
+    JoinType,
+    UnionException,
+    Interval,
+    MySQLQuery,
+)
 
 __author__ = "Timothy Heys"
 __email__ = "theys@kayak.com"
@@ -303,3 +313,9 @@ class UnionTests(unittest.TestCase):
 
         with self.assertRaises(UnionException):
             str(query1 + query2)
+
+    def test_mysql_query_does_not_wrap_unioned_queries_with_params(self):
+        query1 = MySQLQuery.from_(self.table1).select(self.table1.foo)
+        query2 = Query.from_(self.table2).select(self.table2.bar)
+
+        self.assertEqual('SELECT `foo` FROM `abc` UNION SELECT `bar` FROM `efg`', str(query1 + query2))
diff --git a/pypika/tests/test_selects.py b/pypika/tests/test_selects.py
index 45baea3..06e4aba 100644
--- a/pypika/tests/test_selects.py
+++ b/pypika/tests/test_selects.py
@@ -137,7 +137,7 @@ class SelectTests(unittest.TestCase):
     def test_select_with_limit_and_offset(self):
         q1 = Query.from_('abc').select('foo')[10:10]
 
-        self.assertEqual('SELECT "foo" FROM "abc" LIMIT 10 OFFSET 10', str(q1))
+        self.assertEqual('SELECT "foo" FROM "abc" OFFSET 10 LIMIT 10', str(q1))
 
     def test_mysql_query_uses_backtick_quote_chars(self):
         q = MySQLQuery.from_('abc').select('foo', 'bar')