refactoring to use QueryString everwhere instead of raw strings

Author: Daniel Townsend

dev-requirements.txt

@@ -1,3 +1,5 @@
 ipdb==0.11
 ipython==7.1.1
 twine==1.12.1
+mypy==0.650
+

piccolo/columns/base.py

@@ -12,8 +12,9 @@
     NotIn,
     NotLike
 )
-from ..custom_types import Iterable
 from .combination import Where
+from ..custom_types import Iterable
+from ..querystring import QueryString
 
 if t.TYPE_CHECKING:
     from ..table import Table  # noqa
@@ -53,14 +54,6 @@ def not_like(self, value: str) -> Where:
             raise ValueError('% is required for like operators')
         return Where(column=self, value=value, operator=NotLike)
 
-    def format_value(self, value):
-        """
-        Takes the raw Python value and return a string usable in the database
-        query.
-        """
-        value = value if value else 'null'
-        return f'{value}'
-
     def __lt__(self, value) -> Where:
         return Where(column=self, value=value, operator=LessThan)
 
@@ -82,7 +75,8 @@ def __ne__(self, value) -> Where:  # type: ignore
     def __hash__(self):
         return hash(self._name)
 
-    def __str__(self):
+    @property
+    def querystring(self) -> QueryString:
         name = getattr(self, '_name', '')
         column_type = getattr(
             self,
@@ -99,4 +93,7 @@ def __str__(self):
         if references:
             query += f' REFERENCES {references.Meta.tablename}'
 
-        return query
+        return QueryString(query)
+
+    def __str__(self):
+        return self.querystring.__str__()

piccolo/columns/column_types.py

@@ -3,11 +3,12 @@
 import inspect
 import typing as t
 
-from .base import Column
+from piccolo.columns.base import Column
+from piccolo.querystring import Unquoted
 
 if t.TYPE_CHECKING:
     import table  # noqa
-    from ..custom_types import Datetime  # noqa
+    from piccolo.custom_types import Datetime  # noqa
 
 
 class Varchar(Column):
@@ -18,12 +19,6 @@ def __init__(self, length: int = 255, default: str = None,
         self.default = default
         super().__init__(**kwargs)
 
-    def format_value(self, value: str):
-        if not value:
-            return 'null'
-        # TODO sanitize input - use prepared statements
-        return f"'{value}'"
-
 
 class Integer(Column):
 
@@ -47,7 +42,7 @@ def __init__(self, **kwargs) -> None:
             'primary': True,
             'key': True
         })
-        self.default = 'DEFAULT'
+        self.default = Unquoted('DEFAULT')
         super().__init__(**kwargs)
 
 
@@ -57,12 +52,6 @@ def __init__(self, default: 'Datetime' = None, **kwargs) -> None:
         self.default = default
         super().__init__(**kwargs)
 
-    def format_value(self, value: t.Optional[datetime.datetime]):
-        if not value:
-            return 'null'
-        dt_string = value.isoformat().replace('T', ' ')
-        return f"'{dt_string}'"
-
 
 class Boolean(Column):
 

piccolo/columns/combination.py

@@ -1,9 +1,10 @@
-import typing
+import typing as t
 
 from .operators import Operator
 from ..custom_types import Combinable, Iterable
+from piccolo.querystring import QueryString
 
-if typing.TYPE_CHECKING:
+if t.TYPE_CHECKING:
     from .base import Column  # noqa
 
 
@@ -23,11 +24,17 @@ def __init__(self, first: Combinable, second: Combinable) -> None:
         self.first = first
         self.second = second
 
-    def __str__(self):
-        return (
-            f'({self.first.__str__()} {self.operator} {self.second.__str__()})'
+    @property
+    def querystring(self):
+        return QueryString(
+            '({} ' + self.operator + ' {})',
+            self.first.querystring,
+            self.second.querystring
         )
 
+    def __str__(self):
+        self.querystring.__str__()
+
 
 class And(Combination):
     operator = 'AND'
@@ -42,28 +49,42 @@ class Where(CombinableMixin):
     def __init__(
         self,
         column: 'Column',
-        value: typing.Any = None,
+        value: t.Any = None,
         values: Iterable = [],
-        operator: typing.Type[Operator] = None
+        operator: t.Type[Operator] = Operator
     ) -> None:
         self.column = column
         self.value = value
         self.values = values
         self.operator = operator
 
     @property
-    def values_str(self) -> str:
-        return ', '.join(
-            [self.column.format_value(v) for v in self.values]
+    def values_querystring(self) -> QueryString:
+        template = ', '.join(['{}' for i in self.values])
+        return QueryString(
+            template,
+            *self.values
         )
 
-    def __str__(self):
-        kwargs = {
-            'name': self.column._name
-        }
+    @property
+    def querystring(self) -> QueryString:
+        args: t.List[t.Any] = []
         if self.value:
-            kwargs['value'] = self.column.format_value(self.value)
+            args.append(self.value)
         if self.values:
-            kwargs['values'] = self.values_str
+            args.append(self.values_querystring)
 
-        return self.operator.template.format(**kwargs)
+        # TODO Want something cleaner than this.
+        template = self.operator.template.format(
+            name=self.column._name,
+            value='{}',
+            values='{}'
+        )
+
+        return QueryString(
+            template,
+            *args
+        )
+
+    def __str__(self):
+        return self.querystring.__str__()

piccolo/columns/operators.py

@@ -1,5 +1,5 @@
 class Operator():
-    pass
+    template = ''
 
 
 class Equal(Operator):

piccolo/query/base.py

@@ -4,16 +4,22 @@
 
 import ujson as json
 
-from ..utils.sync import run_sync
+from piccolo.querystring import QueryString
+from piccolo.utils.sync import run_sync
 
 if t.TYPE_CHECKING:
     from table import Table  # noqa
 
 
 class Query(object):
 
-    def __init__(self, table: 'Table', base: str = '', *args,
-                 **kwargs) -> None:
+    def __init__(
+        self,
+        table: 'Table',
+        base: QueryString = QueryString(''),
+        *args,
+        **kwargs
+    ) -> None:
         self.base = base
         self.table = table
         super().__init__()
@@ -26,9 +32,13 @@ async def run(self, as_dict=True, in_pool=True):
             )
 
         if in_pool:
-            results = await engine.run_in_pool(self.__str__())
+            results = await engine.run_in_pool(
+                *self.querystring.compile_string()
+            )
         else:
-            results = await engine.run(self.__str__())
+            results = await engine.run(
+                *self.querystring.compile_string()
+            )
 
         if results:
             keys = results[0].keys()
@@ -40,7 +50,7 @@ async def run(self, as_dict=True, in_pool=True):
         if hasattr(self, 'run_callback'):
             self.run_callback(raw)
 
-        # I have multiple ways of modifying the final output
+        # TODO - I have multiple ways of modifying the final output
         # response_handlers, and output ...
         # Might try and merge them.
         raw = self.response_handler(raw)
@@ -88,3 +98,10 @@ def response_handler(self, response):
         the database driver.
         """
         return response
+
+    @property
+    def querystring(self):
+        """
+        Subclasses need to override and return a QueryString object.
+        """
+        raise NotImplementedError()

piccolo/query/methods/alter.py

@@ -1,7 +1,9 @@
 import dataclasses
+import itertools
 
 from piccolo.columns.base import Column
-from ..base import Query
+from piccolo.query.base import Query
+from piccolo.querystring import QueryString
 
 
 @dataclasses.dataclass
@@ -12,8 +14,14 @@ class Rename():
     column: Column
     new_name: str
 
-    def __str__(self):
-        return f' RENAME "{self.column._name}" TO "{self.new_name}"'
+    @property
+    def querystring(self) -> QueryString:
+        return QueryString(
+            f'RENAME {self.column._name} TO {self.new_name}',
+        )
+
+    def __str__(self) -> str:
+        return self.querystring.__str__()
 
 
 @dataclasses.dataclass
@@ -23,8 +31,14 @@ class Drop():
     """
     column: Column
 
-    def __str__(self):
-        return f' DROP "{self.column._name}"'
+    @property
+    def querystring(self) -> QueryString:
+        return QueryString(
+            f'DROP {self.column._name}'
+        )
+
+    def __str__(self) -> str:
+        return self.querystring.__str__()
 
 
 @dataclasses.dataclass
@@ -35,40 +49,63 @@ class Add():
     name: str
     column: Column
 
-    def __str__(self):
+    @property
+    def querystring(self) -> QueryString:
         self.column._name = self.name
-        return f' ADD {self.column.__str__()}'
+        return QueryString(
+            'ADD {}',
+            self.column.querystring
+        )
+
+    def __str__(self) -> str:
+        return self.querystring.__str__()
 
 
 class Alter(Query):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self._add = []
-        self._drop = []
-        self._rename = []
+        self._add: t.List[Add] = []
+        self._drop: t.List[Drop] = []
+        self._rename: t.List[Rename] = []
 
-    def add(self, name: str, column: Column):
+    def add(self, name: str, column: Column) -> 'Alter':
         self._add.append(
             Add(name, column)
         )
         return self
 
-    def rename(self, column: Column, new_name: str):
+    def rename(self, column: Column, new_name: str) -> 'Alter':
         self._rename.append(
             Rename(column, new_name)
         )
         return self
 
-    def drop(self, column: Column):
+    def drop(self, column: Column) -> 'Alter':
         self._drop.append(
             Drop(column)
         )
         return self
 
-    def __str__(self):
-        query = f'ALTER TABLE "{self.table.Meta.tablename}"'
-        for alterations in [self._add, self._rename, self._drop]:
-            for a in alterations:
-                query += a.__str__()
-        return query
+    @property
+    def querystring(self) -> QueryString:
+        query = f'ALTER TABLE {self.table.Meta.tablename}'
+
+        alterations = [
+            i.querystring for i in itertools.chain(
+                self._add,
+                self._rename,
+                self._drop
+            )
+        ]
+
+        for a in alterations:
+            query += ' {}'
+
+        return QueryString(
+            query,
+            *alterations
+        )
+
+    def __str__(self) -> str:
+        return self.querystring.__str__()