changed Interval column implementation for SQLite, and added more tests

Rather than storing the timedelta as a string in SQLite, storing the number of seconds in a Numeric field instead - otherwise WHERE clauses don't work.

Author: dantownsend

piccolo/columns/column_types.py

@@ -651,6 +651,18 @@ def __init__(
         kwargs.update({"default": default})
         super().__init__(**kwargs)
 
+    @property
+    def column_type(self):
+        engine_type = self._meta.table._meta.db.engine_type
+        if engine_type == "postgres":
+            return "INTERVAL"
+        elif engine_type == "sqlite":
+            # We can't use 'INTERVAL' because the type affinity in SQLite would
+            # make it an integer - but we need a numeric field.
+            # https://sqlite.org/datatype3.html#determination_of_column_affinity
+            return "SECONDS"
+        raise Exception("Unrecognized engine type")
+
 
 ###############################################################################
 

piccolo/columns/defaults/interval.py

@@ -53,18 +53,7 @@ def postgres(self):
 
     @property
     def sqlite(self):
-        value = self.get_sqlite_interval_string(
-            attributes=[
-                "weeks",
-                "days",
-                "hours",
-                "minutes",
-                "seconds",
-                "milliseconds",
-                "microseconds",
-            ]
-        ).replace("'", "")
-        return f"'{value}'"
+        return self.timedelta.total_seconds()
 
     def python(self):
         return self.timedelta

piccolo/engine/sqlite.py

@@ -11,7 +11,6 @@
 import aiosqlite
 from aiosqlite import Cursor, Connection
 
-from piccolo.columns.defaults.interval import IntervalCustom
 from piccolo.engine.base import Batch, Engine
 from piccolo.engine.exceptions import TransactionError
 from piccolo.query.base import Query
@@ -56,9 +55,7 @@ def convert_timedelta_in(value):
     """
     Converts the timedelta value being passed into sqlite.
     """
-    return IntervalCustom.from_timedelta(instance=value).sqlite.replace(
-        "'", ""
-    )
+    return value.total_seconds()
 
 
 def convert_numeric_out(value: bytes) -> Decimal:
@@ -93,26 +90,19 @@ def convert_time_out(value: bytes) -> datetime.time:
     return datetime.time.fromisoformat(value.decode("utf8"))
 
 
-def convert_interval_out(value: bytes) -> datetime.timedelta:
+def convert_seconds_out(value: bytes) -> datetime.timedelta:
     """
-    If the value is an interval, convert it to a timedelta instance.
+    If the value is from a seconds column, convert it to a timedelta instance.
     """
-    unit_value_strings = [i.strip() for i in value.decode("utf8").split(",")]
-    kwargs = {}
-
-    for unit_value_string in unit_value_strings:
-        value, key = unit_value_string.split(" ")
-        kwargs[key] = int(value)
-
-    return datetime.timedelta(**kwargs)
+    return datetime.timedelta(seconds=float(value.decode("utf8")))
 
 
 sqlite3.register_converter("Numeric", convert_numeric_out)
 sqlite3.register_converter("Integer", convert_int_out)
 sqlite3.register_converter("UUID", convert_uuid_out)
 sqlite3.register_converter("Date", convert_date_out)
 sqlite3.register_converter("Time", convert_time_out)
-sqlite3.register_converter("Interval", convert_interval_out)
+sqlite3.register_converter("Seconds", convert_seconds_out)
 
 sqlite3.register_adapter(Decimal, convert_numeric_in)
 sqlite3.register_adapter(uuid.UUID, convert_uuid_in)
@@ -343,6 +333,8 @@ def create_db(self, migrate=False):
         else:
             raise Exception(f"Database at {self.path} already exists")
         if migrate:
+            # Commented out for now, as migrations for SQLite aren't as
+            # well supported as Postgres.
             # from piccolo.commands.migration.forwards import (
             #     ForwardsMigrationManager,
             # )

tests/columns/test_interval.py

@@ -22,12 +22,69 @@ def tearDown(self):
         MyTable.alter().drop_table().run_sync()
 
     def test_interval(self):
+        # Test a range of different timedeltas
+        intervals = [
+            datetime.timedelta(weeks=1),
+            datetime.timedelta(days=1),
+            datetime.timedelta(hours=1),
+            datetime.timedelta(minutes=1),
+            datetime.timedelta(seconds=1),
+            datetime.timedelta(milliseconds=1),
+            datetime.timedelta(microseconds=1),
+        ]
+
+        for interval in intervals:
+            # Make sure that saving it works OK.
+            row = MyTable(interval=interval)
+            row.save().run_sync()
+
+            # Make sure that fetching it back works OK.
+            result = (
+                MyTable.objects()
+                .where(MyTable.id == row.id)
+                .first()
+                .run_sync()
+            )
+            self.assertEqual(result.interval, interval)
+
+    def test_interval_where_clause(self):
+        """
+        Make sure a range of where clauses resolve correctly.
+        """
         interval = datetime.timedelta(hours=2)
         row = MyTable(interval=interval)
         row.save().run_sync()
 
-        result = MyTable.objects().first().run_sync()
-        self.assertEqual(result.interval, interval)
+        result = (
+            MyTable.objects()
+            .where(MyTable.interval < datetime.timedelta(hours=3))
+            .first()
+            .run_sync()
+        )
+        self.assertTrue(result is not None)
+
+        result = (
+            MyTable.objects()
+            .where(MyTable.interval < datetime.timedelta(days=1))
+            .first()
+            .run_sync()
+        )
+        self.assertTrue(result is not None)
+
+        result = (
+            MyTable.objects()
+            .where(MyTable.interval > datetime.timedelta(minutes=1))
+            .first()
+            .run_sync()
+        )
+        self.assertTrue(result is not None)
+
+        result = (
+            MyTable.exists()
+            .where(MyTable.interval == datetime.timedelta(hours=2))
+            .run_sync()
+        )
+        self.assertTrue(result)
 
 
 class TestIntervalDefault(TestCase):