disable batching for sqlalchemy < 1.2

Author: jnak

Diff for: graphene_sqlalchemy/tests/test_batching.py

@@ -1,6 +1,9 @@
 import contextlib
 import logging
 
+import pkg_resources
+import pytest
+
 import graphene
 
 from ..types import SQLAlchemyObjectType
@@ -78,6 +81,14 @@ def resolve_reporters(self, _info):
     return graphene.Schema(query=Query)
 
 
+def is_sqlalchemy_version_less_than(version_string):
+    return pkg_resources.get_distribution('SQLAlchemy').parsed_version < pkg_resources.parse_version(version_string)
+
+
+if is_sqlalchemy_version_less_than('1.2'):
+    pytest.skip('SQL batching only works for SQLAlchemy 1.2+', allow_module_level=True)
+
+
 def test_many_to_one(session_factory):
     session = session_factory()
     make_fixture(session)
@@ -99,6 +110,13 @@ def test_many_to_one(session_factory):
         messages = sqlalchemy_logging_handler.messages
 
     assert len(messages) == 5
+
+    if is_sqlalchemy_version_less_than('1.3'):
+        # The batched SQL statement generated is different in 1.2.x
+        # SQLAlchemy 1.3+ optimizes out a JOIN statement in `selectin`
+        # See https://git.io/JewQu
+        return
+
     assert messages == [
       'BEGIN (implicit)',
 
@@ -161,6 +179,13 @@ def test_one_to_one(session_factory):
         messages = sqlalchemy_logging_handler.messages
 
     assert len(messages) == 5
+
+    if is_sqlalchemy_version_less_than('1.3'):
+        # The batched SQL statement generated is different in 1.2.x
+        # SQLAlchemy 1.3+ optimizes out a JOIN statement in `selectin`
+        # See https://git.io/JewQu
+        return
+
     assert messages == [
       'BEGIN (implicit)',
 

Diff for: graphene_sqlalchemy/types.py

@@ -4,10 +4,9 @@
 from promise import dataloader, promise
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import (ColumnProperty, CompositeProperty,
-                            RelationshipProperty, Session)
+                            RelationshipProperty, Session, strategies)
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.orm.query import QueryContext
-from sqlalchemy.orm.strategies import SelectInLoader
 
 from graphene import Field
 from graphene.relay import Connection, Node
@@ -213,6 +212,8 @@ def _get_custom_resolver(obj_type, orm_field_name):
 def _get_relationship_resolver(obj_type, relationship_prop, model_attr):
     """
     Batch SQL queries using Dataloader to avoid the N+1 problem.
+    SQL batching only works for SQLAlchemy 1.2+ since it depends on
+    the `selectin` loader.
 
     :param SQLAlchemyObjectType obj_type:
     :param sqlalchemy.orm.properties.RelationshipProperty relationship_prop:
@@ -222,7 +223,7 @@ def _get_relationship_resolver(obj_type, relationship_prop, model_attr):
     child_mapper = relationship_prop.mapper
     parent_mapper = relationship_prop.parent
 
-    if relationship_prop.uselist:
+    if not getattr(strategies, 'SelectInLoader', None) or relationship_prop.uselist:
         # TODO Batch many-to-many and one-to-many relationships
         return _get_attr_resolver(obj_type, model_attr, model_attr)
 
@@ -239,14 +240,18 @@ def batch_load_fn(self, parents):  # pylint: disable=method-hidden
             than re-implementing and maintainnig a big chunk of the `selectin`
             loader logic ourselves.
 
-            The approach is to here to build a regular query that
+            The approach here is to build a regular query that
             selects the parent and `selectin` load the relationship.
             But instead of having the query emits 2 `SELECT` statements
             when callling `all()`, we skip the first `SELECT` statement
             and jump right before the `selectin` loader is called.
             To accomplish this, we have to construct objects that are
             normally built in the first part of the query in order
             to call directly `SelectInLoader._load_for_path`.
+
+            TODO Move this logic to a util in the SQLAlchemy repo as per
+              SQLAlchemy's main maitainer suggestion.
+              See https://git.io/JewQ7
             """
             session = Session.object_session(parents[0])
 
@@ -258,7 +263,7 @@ def batch_load_fn(self, parents):  # pylint: disable=method-hidden
                 # The behavior of `selectin` is undefined if the parent is dirty
                 assert parent not in session.dirty
 
-            loader = SelectInLoader(relationship_prop, (('lazy', 'selectin'),))
+            loader = strategies.SelectInLoader(relationship_prop, (('lazy', 'selectin'),))
 
             # Should the boolean be set to False? Does it matter for our purposes?
             states = [(sqlalchemy.inspect(parent), True) for parent in parents]

Diff for: setup.cfg

@@ -9,7 +9,7 @@ max-line-length = 120
 no_lines_before=FIRSTPARTY
 known_graphene=graphene,graphql_relay,flask_graphql,graphql_server,sphinx_graphene_theme
 known_first_party=graphene_sqlalchemy
-known_third_party=app,database,flask,mock,models,nameko,promise,pytest,schema,setuptools,singledispatch,six,sqlalchemy,sqlalchemy_utils
+known_third_party=app,database,flask,mock,models,nameko,pkg_resources,promise,pytest,schema,setuptools,singledispatch,six,sqlalchemy,sqlalchemy_utils
 sections=FUTURE,STDLIB,THIRDPARTY,GRAPHENE,FIRSTPARTY,LOCALFOLDER
 skip_glob=examples/nameko_sqlalchemy