improving performance of Column copying

Author: dantownsend

piccolo/columns/base.py

@@ -90,6 +90,21 @@ def resolved_references(self) -> t.Type[Table]:
                 "LazyTableReference instance."
             )
 
+    def copy(self) -> ForeignKeyMeta:
+        kwargs = self.__dict__.copy()
+        kwargs.update(proxy_columns=self.proxy_columns.copy())
+        return self.__class__(**kwargs)
+
+    def __copy__(self) -> ForeignKeyMeta:
+        return self.copy()
+
+    def __deepcopy__(self, memo) -> ForeignKeyMeta:
+        """
+        We override deepcopy, as it's too slow if it has to recreate
+        everything.
+        """
+        return self.copy()
+
 
 @dataclass
 class ColumnMeta:
@@ -164,6 +179,23 @@ def get_full_name(self, just_alias=False) -> str:
         else:
             return f'{alias} AS "{column_name}"'
 
+    def copy(self) -> ColumnMeta:
+        kwargs = self.__dict__.copy()
+        kwargs.update(
+            params=self.params.copy(), call_chain=self.call_chain.copy(),
+        )
+        return self.__class__(**kwargs)
+
+    def __copy__(self) -> ColumnMeta:
+        return self.copy()
+
+    def __deepcopy__(self, memo) -> ColumnMeta:
+        """
+        We override deepcopy, as it's too slow if it has to recreate
+        everything.
+        """
+        return self.copy()
+
 
 class Selectable(metaclass=ABCMeta):
     @abstractmethod
@@ -455,6 +487,18 @@ def querystring(self) -> QueryString:
 
         return QueryString(query)
 
+    def copy(self) -> Column:
+        column: Column = copy.copy(self)
+        column._meta = self._meta.copy()
+        return column
+
+    def __deepcopy__(self, memo) -> Column:
+        """
+        We override deepcopy, as it's too slow if it has to recreate
+        everything.
+        """
+        return self.copy()
+
     def __str__(self):
         return self.querystring.__str__()
 

piccolo/columns/column_types.py

@@ -6,7 +6,12 @@
 import uuid
 from datetime import date, datetime, time, timedelta
 
-from piccolo.columns.base import Column, ForeignKeyMeta, OnDelete, OnUpdate
+from piccolo.columns.base import (
+    Column,
+    ForeignKeyMeta,
+    OnDelete,
+    OnUpdate,
+)
 from piccolo.columns.defaults.date import DateArg, DateCustom, DateNow
 from piccolo.columns.defaults.interval import IntervalArg, IntervalCustom
 from piccolo.columns.defaults.time import TimeArg, TimeCustom, TimeNow
@@ -1041,6 +1046,12 @@ def __init__(
                     Table, OnDelete.cascade, OnUpdate.cascade
                 )
 
+    def copy(self) -> ForeignKey:
+        column: ForeignKey = copy.copy(self)
+        column._meta = self._meta.copy()
+        column._foreign_key_meta = self._foreign_key_meta.copy()
+        return column
+
     def set_proxy_columns(self):
         """
         In order to allow a fluent interface, where tables can be traversed
@@ -1050,7 +1061,7 @@ def set_proxy_columns(self):
         """
         _foreign_key_meta = object.__getattribute__(self, "_foreign_key_meta")
         for column in _foreign_key_meta.resolved_references._meta.columns:
-            _column: Column = copy.deepcopy(column)
+            _column: Column = column.copy()
             setattr(self, _column._meta.name, _column)
             _foreign_key_meta.proxy_columns.append(_column)
 
@@ -1081,11 +1092,12 @@ def __getattribute__(self, name: str):
         except AttributeError:
             raise AttributeError
 
-        foreignkey_class = object.__getattribute__(self, "__class__")
+        foreignkey_class: t.Type[ForeignKey] = object.__getattribute__(
+            self, "__class__"
+        )
 
         if isinstance(value, foreignkey_class):  # i.e. a ForeignKey
-            new_column = copy.deepcopy(value)
-            new_column._meta.call_chain = copy.copy(self._meta.call_chain)
+            new_column = value.copy()
             new_column._meta.call_chain.append(self)
 
             # We have to set limits to the call chain because Table 1 can
@@ -1094,7 +1106,7 @@ def __getattribute__(self, name: str):
             # When querying a call chain more than 10 levels deep, an error
             # will be raised. Often there are more effective ways of
             # structuring a query than joining so many tables anyway.
-            if len(new_column._meta.call_chain) > 10:
+            if len(new_column._meta.call_chain) >= 10:
                 raise Exception("Call chain too long!")
 
             for proxy_column in self._foreign_key_meta.proxy_columns:
@@ -1106,10 +1118,10 @@ def __getattribute__(self, name: str):
             for (
                 column
             ) in value._foreign_key_meta.resolved_references._meta.columns:
-                _column: Column = copy.deepcopy(column)
-                _column._meta.call_chain = copy.copy(
-                    new_column._meta.call_chain
-                )
+                _column: Column = column.copy()
+                _column._meta.call_chain = [
+                    i for i in new_column._meta.call_chain
+                ]
                 _column._meta.call_chain.append(new_column)
                 if _column._meta.name == "id":
                     continue
@@ -1118,8 +1130,8 @@ def __getattribute__(self, name: str):
 
             return new_column
         elif issubclass(type(value), Column):
-            new_column = copy.deepcopy(value)
-            new_column._meta.call_chain = copy.copy(self._meta.call_chain)
+            new_column = value.copy()
+            new_column._meta.call_chain = self._meta.call_chain.copy()
             new_column._meta.call_chain.append(self)
             return new_column
         else:

piccolo/table.py

@@ -1,5 +1,4 @@
 from __future__ import annotations
-import copy
 from dataclasses import dataclass, field
 import inspect
 import itertools
@@ -435,7 +434,7 @@ def ref(cls, column_name: str) -> Column:
             reference_column_name
         )
 
-        _reference_column = copy.deepcopy(reference_column)
+        _reference_column = reference_column.copy()
         _reference_column._meta.name = (
             f"{local_column_name}.{reference_column_name}"
         )

tests/columns/test_base.py

@@ -25,3 +25,17 @@ def test_like_raises(self):
         for arg in ["%guido", "guido%", "%guido%"]:
             column.like("%foo")
             column.ilike("foo%")
+
+
+class TestCopy(TestCase):
+    def test_copy(self):
+        """
+        Try copying columns.
+        """
+        column = MyTable.name
+        new_column = column.copy()
+        self.assertNotEqual(id(column), id(new_column))
+        self.assertNotEqual(id(column._meta), id(new_column._meta))
+        self.assertNotEqual(
+            id(column._meta.call_chain), id(new_column._meta.call_chain)
+        )

tests/columns/test_foreignkey.py

@@ -1,7 +1,8 @@
 from unittest import TestCase
+import time
 
 from piccolo.table import Table
-from piccolo.columns import ForeignKey, Varchar, LazyTableReference
+from piccolo.columns import Column, ForeignKey, Varchar, LazyTableReference
 
 
 class Manager(Table):
@@ -117,3 +118,25 @@ def test_attribute_access(self):
         self.assertTrue(isinstance(Band1.manager.name, Varchar))
         self.assertTrue(isinstance(Band2.manager.name, Varchar))
         self.assertTrue(isinstance(Band3.manager.name, Varchar))
+
+    def test_recursion_limit(self):
+        """
+        When a table has a ForeignKey to itself, an Exception should be raised
+        if the call chain is too large.
+        """
+        # Should be fine:
+        column: Column = Manager.manager.name
+        self.assertTrue(len(column._meta.call_chain), 1)
+        self.assertTrue(isinstance(column, Varchar))
+
+        with self.assertRaises(Exception):
+            Manager.manager.manager.manager.manager.manager.manager.manager.manager.manager.manager.manager.name  # noqa
+
+    def test_recursion_time(self):
+        """
+        Make sure that a really large call chain doesn't take too long.
+        """
+        start = time.time()
+        Manager.manager.manager.manager.manager.manager.manager.name
+        end = time.time()
+        self.assertTrue(end - start < 1.0)