merge from default

11 files changed, 225 insertions, 44 deletions

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 8951217ff..218700ba8 100644
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -7,6 +7,23 @@
     :version: 0.8.0
 
     .. change::
+        :tags: bug, orm
+        :tickets: 2662
+
+      A clear error message is emitted if an event handler
+      attempts to emit SQL on a Session within the after_commit()
+      handler, where there is not a viable transaction in progress.
+
+    .. change::
+        :tags: bug, orm
+        :tickets: 2665
+
+      Detection of a primary key change within the process
+      of cascading a natural primary key update will succeed
+      even if the key is composite and only some of the
+      attributes have changed.
+
+    .. change::
         :tags: feature, orm
         :tickets: 2658
 
diff --git a/doc/build/orm/mapper_config.rst b/doc/build/orm/mapper_config.rst
index 459a1dee6..2560c6f41 100644
--- a/doc/build/orm/mapper_config.rst
+++ b/doc/build/orm/mapper_config.rst
@@ -992,8 +992,11 @@ subquery::
                 orders.c.customer_id
                 ]).group_by(orders.c.customer_id).alias()
 
-    customer_select = select([customers,subq]).\
-                where(customers.c.customer_id==subq.c.customer_id)
+    customer_select = select([customers, subq]).\
+                select_from(
+                    join(customers, subq,
+                            customers.c.id == subq.c.customer_id)
+                ).alias()
 
     class Customer(Base):
         __table__ = customer_select
@@ -1011,6 +1014,19 @@ primary key of the ``orders`` table is not represented in the mapping;  the ORM
 will only emit an INSERT into a table for which it has mapped the primary
 key.
 
+.. note::
+
+    The practice of mapping to arbitrary SELECT statements, especially
+    complex ones as above, is
+    almost never needed; it necessarily tends to produce complex queries
+    which are often less efficient than that which would be produced
+    by direct query construction.   The practice is to some degree
+    based on the very early history of SQLAlchemy where the :func:`.mapper`
+    construct was meant to represent the primary querying interface;
+    in modern usage, the :class:`.Query` object can be used to construct
+    virtually any SELECT statement, including complex composites, and should
+    be favored over the "map-to-selectable" approach.
+
 Multiple Mappers for One Class
 ==============================
 
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 5a0f2ab4c..ee2f0134a 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -39,7 +39,6 @@ def _as_declarative(cls, classname, dict_):
     mapper_args_fn = None
     table_args = inherited_table_args = None
     tablename = None
-    parent_columns = ()
 
     declarative_props = (declared_attr, util.classproperty)
 
@@ -57,8 +56,6 @@ def _as_declarative(cls, classname, dict_):
                 return
 
         class_mapped = _declared_mapping_info(base) is not None
-        if class_mapped:
-            parent_columns = base.__table__.c.keys()
 
         for name, obj in vars(base).items():
             if name == '__mapper_args__':
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 93a84fb9b..c9385daaa 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -1145,7 +1145,7 @@ class History(History):
 
         from sqlalchemy import inspect
 
-        hist = inspect(myobject).attr.myattribute.history
+        hist = inspect(myobject).attrs.myattribute.history
 
     Each tuple member is an iterable sequence:
 
diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index 5b66c72ff..cea07bcf0 100644
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -1118,41 +1118,103 @@ class SessionEvents(event.Events):
     def after_transaction_create(self, session, transaction):
         """Execute when a new :class:`.SessionTransaction` is created.
 
+        This event differs from :meth:`~.SessionEvents.after_begin`
+        in that it occurs for each :class:`.SessionTransaction`
+        overall, as opposed to when transactions are begun
+        on individual database connections.  It is also invoked
+        for nested transactions and subtransactions, and is always
+        matched by a corresponding
+        :meth:`~.SessionEvents.after_transaction_end` event
+        (assuming normal operation of the :class:`.Session`).
+
         :param session: the target :class:`.Session`.
         :param transaction: the target :class:`.SessionTransaction`.
 
         .. versionadded:: 0.8
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.after_transaction_end`
+
         """
 
     def after_transaction_end(self, session, transaction):
         """Execute when the span of a :class:`.SessionTransaction` ends.
 
+        This event differs from :meth:`~.SessionEvents.after_commit`
+        in that it corresponds to all :class:`.SessionTransaction`
+        objects in use, including those for nested transactions
+        and subtransactions, and is always matched by a corresponding
+        :meth:`~.SessionEvents.after_transaction_create` event.
+
         :param session: the target :class:`.Session`.
         :param transaction: the target :class:`.SessionTransaction`.
 
         .. versionadded:: 0.8
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.after_transaction_create`
+
         """
 
     def before_commit(self, session):
         """Execute before commit is called.
 
-        Note that this may not be per-flush if a longer running
-        transaction is ongoing.
+        .. note::
+
+            The :meth:`.before_commit` hook is *not* per-flush,
+            that is, the :class:`.Session` can emit SQL to the database
+            many times within the scope of a transaction.
+            For interception of these events, use the :meth:`~.SessionEvents.before_flush`,
+            :meth:`~.SessionEvents.after_flush`, or :meth:`~.SessionEvents.after_flush_postexec`
+            events.
 
         :param session: The target :class:`.Session`.
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.after_commit`
+
+            :meth:`~.SessionEvents.after_begin`
+
+            :meth:`~.SessionEvents.after_transaction_create`
+
+            :meth:`~.SessionEvents.after_transaction_end`
+
         """
 
     def after_commit(self, session):
         """Execute after a commit has occurred.
 
-        Note that this may not be per-flush if a longer running
-        transaction is ongoing.
+        .. note::
+
+            The :meth:`~.SessionEvents.after_commit` hook is *not* per-flush,
+            that is, the :class:`.Session` can emit SQL to the database
+            many times within the scope of a transaction.
+            For interception of these events, use the :meth:`~.SessionEvents.before_flush`,
+            :meth:`~.SessionEvents.after_flush`, or :meth:`~.SessionEvents.after_flush_postexec`
+            events.
+
+        .. note::
+
+            The :class:`.Session` is not in an active tranasction
+            when the :meth:`~.SessionEvents.after_commit` event is invoked, and therefore
+            can not emit SQL.  To emit SQL corresponding to every transaction,
+            use the :meth:`~.SessionEvents.before_commit` event.
 
         :param session: The target :class:`.Session`.
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.before_commit`
+
+            :meth:`~.SessionEvents.after_begin`
+
+            :meth:`~.SessionEvents.after_transaction_create`
+
+            :meth:`~.SessionEvents.after_transaction_end`
+
         """
 
     def after_rollback(self, session):
@@ -1211,6 +1273,12 @@ class SessionEvents(event.Events):
          objects which can be passed to the :meth:`.Session.flush` method
          (note this usage is deprecated).
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.after_flush`
+
+            :meth:`~.SessionEvents.after_flush_postexec`
+
         """
 
     def after_flush(self, session, flush_context):
@@ -1225,6 +1293,12 @@ class SessionEvents(event.Events):
         :param flush_context: Internal :class:`.UOWTransaction` object
          which handles the details of the flush.
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.before_flush`
+
+            :meth:`~.SessionEvents.after_flush_postexec`
+
         """
 
     def after_flush_postexec(self, session, flush_context):
@@ -1239,6 +1313,14 @@ class SessionEvents(event.Events):
         :param session: The target :class:`.Session`.
         :param flush_context: Internal :class:`.UOWTransaction` object
          which handles the details of the flush.
+
+
+        .. seealso::
+
+            :meth:`~.SessionEvents.before_flush`
+
+            :meth:`~.SessionEvents.after_flush`
+
         """
 
     def after_begin(self, session, transaction, connection):
@@ -1249,6 +1331,16 @@ class SessionEvents(event.Events):
         :param connection: The :class:`~.engine.Connection` object
          which will be used for SQL statements.
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.before_commit`
+
+            :meth:`~.SessionEvents.after_commit`
+
+            :meth:`~.SessionEvents.after_transaction_create`
+
+            :meth:`~.SessionEvents.after_transaction_end`
+
         """
 
     def before_attach(self, session, instance):
@@ -1262,6 +1354,10 @@ class SessionEvents(event.Events):
            :meth:`.before_attach` is provided for those cases where
            the item should not yet be part of the session state.
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.after_attach`
+
         """
 
     def after_attach(self, session, instance):
@@ -1280,6 +1376,10 @@ class SessionEvents(event.Events):
            yet complete) consider the
            new :meth:`.before_attach` event.
 
+        .. seealso::
+
+            :meth:`~.SessionEvents.before_attach`
+
         """
 
     def after_bulk_update(self, session, query, query_context, result):
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 2915fd4c8..5fb9b514d 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -57,6 +57,10 @@ class _SessionClassMethods(object):
         return object_session(instance)
 
 
+ACTIVE = util.symbol('ACTIVE')
+PREPARED = util.symbol('PREPARED')
+DEACTIVE = util.symbol('DEACTIVE')
+
 class SessionTransaction(object):
     """A :class:`.Session`-level transaction.
 
@@ -144,8 +148,7 @@ class SessionTransaction(object):
         self._connections = {}
         self._parent = parent
         self.nested = nested
-        self._active = True
-        self._prepared = False
+        self._state = ACTIVE
         if not parent and nested:
             raise sa_exc.InvalidRequestError(
                 "Can't start a SAVEPOINT transaction when no existing "
@@ -159,11 +162,17 @@ class SessionTransaction(object):
 
     @property
     def is_active(self):
-        return self.session is not None and self._active
+        return self.session is not None and self._state is ACTIVE
 
     def _assert_is_active(self):
         self._assert_is_open()
-        if not self._active:
+        if self._state is PREPARED:
+            raise sa_exc.InvalidRequestError(
+                    "This session is in 'prepared' state, where no further "
+                    "SQL can be emitted until the transaction is fully "
+                    "committed."
+                )
+        elif self._state is DEACTIVE:
             if self._rollback_exception:
                 raise sa_exc.InvalidRequestError(
                     "This Session's transaction has been rolled back "
@@ -327,12 +336,11 @@ class SessionTransaction(object):
                 self.rollback()
                 raise
 
-        self._deactivate()
-        self._prepared = True
+        self._state = PREPARED
 
     def commit(self):
         self._assert_is_open()
-        if not self._prepared:
+        if self._state is not PREPARED:
             self._prepare_impl()
 
         if self._parent is None or self.nested:
@@ -355,14 +363,14 @@ class SessionTransaction(object):
             for subtransaction in stx._iterate_parents(upto=self):
                 subtransaction.close()
 
-        if self.is_active or self._prepared:
+        if self._state in (ACTIVE, PREPARED):
             for transaction in self._iterate_parents():
                 if transaction._parent is None or transaction.nested:
                     transaction._rollback_impl()
-                    transaction._deactivate()
+                    transaction._state = DEACTIVE
                     break
                 else:
-                    transaction._deactivate()
+                    transaction._state = DEACTIVE
 
         sess = self.session
 
@@ -393,9 +401,6 @@ class SessionTransaction(object):
 
         self.session.dispatch.after_rollback(self.session)
 
-    def _deactivate(self):
-        self._active = False
-
     def close(self):
         self.session.transaction = self._parent
         if self._parent is None:
@@ -406,7 +411,7 @@ class SessionTransaction(object):
                 else:
                     transaction.close()
 
-        self._deactivate()
+        self._state = DEACTIVE
         if self.session.dispatch.after_transaction_end:
             self.session.dispatch.after_transaction_end(self.session, self)
 
diff --git a/lib/sqlalchemy/orm/sync.py b/lib/sqlalchemy/orm/sync.py
index b386a6531..6524ab27a 100644
--- a/lib/sqlalchemy/orm/sync.py
+++ b/lib/sqlalchemy/orm/sync.py
@@ -94,7 +94,8 @@ def source_modified(uowcommit, source, source_mapper, synchronize_pairs):
             _raise_col_to_prop(False, source_mapper, l, None, r)
         history = uowcommit.get_attribute_history(source, prop.key,
                                         attributes.PASSIVE_NO_INITIALIZE)
-        return bool(history.deleted)
+        if bool(history.deleted):
+            return True
     else:
         return False
 
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 90837f4ab..490004e39 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -2604,7 +2604,14 @@ class FromClause(Selectable):
                     **params)
 
     def select(self, whereclause=None, **params):
-        """return a SELECT of this :class:`.FromClause`."""
+        """return a SELECT of this :class:`.FromClause`.
+
+        .. seealso::
+
+            :func:`~.sql.expression.select` - general purpose
+            method which allows for arbitrary column lists.
+
+        """
 
         return select([self], whereclause, **params)
 
diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index 0a50a48cb..5c8968842 100644
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -780,6 +780,27 @@ class SessionStateTest(_fixtures.FixtureTest):
         go()
         eq_(canary, [False])
 
+    def test_deleted_expunged(self):
+        users, User = self.tables.users, self.classes.User
+
+        mapper(User, users)
+        sess = Session()
+        sess.add(User(name='x'))
+        sess.commit()
+
+        u1 = sess.query(User).first()
+        sess.delete(u1)
+
+        assert not was_deleted(u1)
+        sess.flush()
+
+        assert was_deleted(u1)
+        assert u1 not in sess
+        assert object_session(u1) is sess
+        sess.commit()
+
+        assert object_session(u1) is None
+
 class SessionStateWFixtureTest(_fixtures.FixtureTest):
 
     def test_autoflush_rollback(self):
@@ -835,24 +856,6 @@ class SessionStateWFixtureTest(_fixtures.FixtureTest):
             assert sa.orm.object_session(a) is None
             assert sa.orm.attributes.instance_state(a).session_id is None
 
-    def test_deleted_expunged(self):
-        users, User = self.tables.users, self.classes.User
-
-        mapper(User, users)
-        sess = Session()
-
-        u1 = sess.query(User).first()
-        sess.delete(u1)
-
-        assert not was_deleted(u1)
-        sess.flush()
-
-        assert was_deleted(u1)
-        assert u1 not in sess
-        assert object_session(u1) is sess
-        sess.commit()
-
-        assert object_session(u1) is None
 
 
 class WeakIdentityMapTest(_fixtures.FixtureTest):
diff --git a/test/orm/test_sync.py b/test/orm/test_sync.py
index a2c894725..c5825e88b 100644
--- a/test/orm/test_sync.py
+++ b/test/orm/test_sync.py
@@ -212,6 +212,29 @@ class SyncTest(fixtures.MappedTest,
             True
         )
 
+    def test_source_modified_composite(self):
+        uowcommit, a1, b1, a_mapper, b_mapper = self._fixture()
+        a1.obj().foo = 10
+        a1._commit_all(a1.dict)
+        a1.obj().foo = 12
+        pairs = [(a_mapper.c.id, b_mapper.c.id,),
+                (a_mapper.c.foo, b_mapper.c.id)]
+        eq_(
+            sync.source_modified(uowcommit, a1, a_mapper, pairs),
+            True
+        )
+
+    def test_source_modified_composite_unmodified(self):
+        uowcommit, a1, b1, a_mapper, b_mapper = self._fixture()
+        a1.obj().foo = 10
+        a1._commit_all(a1.dict)
+        pairs = [(a_mapper.c.id, b_mapper.c.id,),
+                (a_mapper.c.foo, b_mapper.c.id)]
+        eq_(
+            sync.source_modified(uowcommit, a1, a_mapper, pairs),
+            False
+        )
+
     def test_source_modified_no_unmapped(self):
         uowcommit, a1, b1, a_mapper, b_mapper = self._fixture()
         pairs = [(b_mapper.c.id, b_mapper.c.id,)]
diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py
index 28735bd11..7df6ecf91 100644
--- a/test/orm/test_transaction.py
+++ b/test/orm/test_transaction.py
@@ -358,6 +358,18 @@ class SessionTransactionTest(FixtureTest):
                               sess.begin, subtransactions=True)
         sess.close()
 
+    def test_no_sql_during_prepare(self):
+        sess = create_session(bind=testing.db, autocommit=False)
+
+        @event.listens_for(sess, "after_commit")
+        def go(session):
+            session.execute("select 1")
+        assert_raises_message(sa_exc.InvalidRequestError,
+                    "This session is in 'prepared' state, where no "
+                    "further SQL can be emitted until the "
+                    "transaction is fully committed.",
+                    sess.commit)
+
     def _inactive_flushed_session_fixture(self):
         users, User = self.tables.users, self.classes.User