Skip to content

Commit

Permalink
improving performance of Column copying
Browse files Browse the repository at this point in the history
  • Loading branch information
dantownsend committed Jan 1, 2021
1 parent 284854b commit 46b5741
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 15 deletions.
44 changes: 44 additions & 0 deletions piccolo/columns/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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__()

Expand Down
36 changes: 24 additions & 12 deletions piccolo/columns/column_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions piccolo/table.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from __future__ import annotations
import copy
from dataclasses import dataclass, field
import inspect
import itertools
Expand Down Expand Up @@ -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}"
)
Expand Down
14 changes: 14 additions & 0 deletions tests/columns/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
25 changes: 24 additions & 1 deletion tests/columns/test_foreignkey.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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)

0 comments on commit 46b5741

Please sign in to comment.