Merge pull request #18 from kvesteri/hotfix/17-slow-includes

Fix slow includes, closes #17

.travis.yml

@@ -7,13 +7,15 @@ before_script:
 language: python
 python:
   - 2.7
-  - 3.3
   - 3.4
   - 3.5
+  - 3.6
 env:
   matrix:
     - SQLALCHEMY=SQLAlchemy>=1.0,<1.1
-    - SQLALCHEMY=SQLAlchemy>=1.1
+    - SQLALCHEMY=SQLAlchemy>=1.1,<1.2
+    - SQLALCHEMY=SQLAlchemy>=1.2,<1.3
+    - SQLALCHEMY=SQLAlchemy>=1.3
 
 install:
   - "pip install $SQLALCHEMY"

setup.py

@@ -64,9 +64,9 @@ def get_version():
         'Programming Language :: Python :: 2',
         'Programming Language :: Python :: 2.7',
         'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.3',
         'Programming Language :: Python :: 3.4',
         'Programming Language :: Python :: 3.5',
+        'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: Implementation :: CPython',
         'Programming Language :: Python :: Implementation :: PyPy',
         'Topic :: Internet :: WWW/HTTP :: Dynamic Content',

sqlalchemy_json_api/__init__.py

@@ -5,6 +5,7 @@
     UnknownFieldKey,
     UnknownModel
 )
+from .hybrids import CompositeId  # noqa
 from .query_builder import QueryBuilder, RESERVED_KEYWORDS  # noqa
 from .utils import assert_json_document  # noqa
 

sqlalchemy_json_api/hybrids.py

@@ -0,0 +1,32 @@
+import sqlalchemy as sa
+from sqlalchemy.ext.hybrid import Comparator
+
+
+class CompositeId(Comparator):
+    def __init__(self, keys, separator=':', label='id'):
+        self.keys = keys
+        self.separator = separator
+        self.label = label
+
+    def operate(self, op, other):
+        if isinstance(other, sa.sql.selectable.Select):
+            return op(sa.tuple_(*self.keys), other)
+        if not isinstance(other, CompositeId):
+            other = CompositeId(other)
+        return sa.and_(
+            op(key, other_key)
+            for key, other_key in zip(self.keys, other.keys)
+        )
+
+    def __clause_element__(self):
+        parts = [self.keys[0]]
+        for key in self.keys[1:]:
+            parts.append(sa.text("'{}'".format(self.separator)))
+            parts.append(key)
+        return sa.func.concat(*parts).label(self.label)
+
+    def __str__(self):
+        return self.separator.join(str(k) for k in self.keys)
+
+    def __repr__(self):
+        return '<CompositeId {}>'.format(repr(self.keys))

sqlalchemy_json_api/query_builder.py

@@ -22,6 +22,7 @@
     UnknownFieldKey,
     UnknownModel
 )
+from .hybrids import CompositeId
 from .utils import (
     adapt,
     chain_if,
@@ -840,23 +841,39 @@ def build_single_included(self, fields, path):
         relationships = path_to_relationships(path, self.model)
 
         cls = relationships[-1].mapper.class_
-        alias = sa.orm.aliased(cls)
-        expr = self.build_included_json_object(alias, fields)
-        query = select_correlated_expression(
+        subalias = sa.orm.aliased(cls)
+        subquery = select_correlated_expression(
             self.model,
-            expr,
+            subalias.id,
             path,
-            alias,
+            subalias,
             self.from_obj,
             correlate=False
-        ).distinct()
+        ).with_only_columns(split_if_composite(subalias.id)).distinct()
+
+        alias = sa.orm.aliased(cls)
+        expr = self.build_included_json_object(alias, fields)
+        query = sa.select(
+            [expr],
+            from_obj=alias
+        ).where(alias.id.in_(subquery)).distinct()
+
         if cls is self.model:
             query = query.where(
                 alias.id.notin_(
                     sa.select(
-                        [get_attrs(self.from_obj).id],
+                        split_if_composite(get_attrs(self.from_obj).id),
                         from_obj=self.from_obj
                     )
                 )
             )
         return query
+
+
+def split_if_composite(column):
+    if (
+        hasattr(column.comparator, 'expression') and
+        isinstance(column.comparator.expression, CompositeId)
+    ):
+        return column.comparator.expression.keys
+    return [column]

tests/conftest.py

@@ -4,42 +4,14 @@
 import sqlalchemy as sa
 from sqlalchemy import create_engine
 from sqlalchemy.ext.declarative import declarative_base
-from sqlalchemy.ext.hybrid import Comparator, hybrid_property
+from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import sessionmaker
 
-from sqlalchemy_json_api import QueryBuilder
+from sqlalchemy_json_api import CompositeId, QueryBuilder
 
 warnings.filterwarnings('error')
 
 
-class CompositeId(Comparator):
-    def __init__(self, keys, separator=':', label='id'):
-        self.keys = keys
-        self.separator = separator
-        self.label = label
-
-    def operate(self, op, other):
-        if not isinstance(other, CompositeId):
-            other = CompositeId(other)
-        return sa.and_(
-            op(key, other_key)
-            for key, other_key in zip(self.keys, other.keys)
-        )
-
-    def __clause_element__(self):
-        parts = [self.keys[0]]
-        for key in self.keys[1:]:
-            parts.append(sa.text("'{}'".format(self.separator)))
-            parts.append(key)
-        return sa.func.concat(*parts).label(self.label)
-
-    def __str__(self):
-        return self.separator.join(str(k) for k in self.keys)
-
-    def __repr__(self):
-        return '<CompositeId {}>'.format(repr(self.keys))
-
-
 @pytest.fixture(scope='class')
 def base():
     return declarative_base()

tests/test_select_with_include.py

@@ -60,6 +60,29 @@ def test_with_many_root_entities(
                     ]
                 }
             ),
+            (
+                {
+                    'comments': [],
+                    'users': [],
+                    'memberships': []
+                },
+                ['author.memberships'],
+                {
+                    'included': [
+                        {'type': 'memberships', 'id': '1:1'},
+                        {'type': 'memberships', 'id': '2:1'},
+                        {'type': 'memberships', 'id': '3:1'},
+                        {'type': 'users', 'id': '1'},
+                        {'type': 'users', 'id': '2'}
+                    ],
+                    'data': [
+                        {'type': 'comments', 'id': '1'},
+                        {'type': 'comments', 'id': '2'},
+                        {'type': 'comments', 'id': '3'},
+                        {'type': 'comments', 'id': '4'}
+                    ]
+                }
+            ),
         )
     )
     def test_fetches_distinct_included_resources(