- The "allow_null_pks" flag is now called "allow_partial_pks",

defaults to True, acts like it did in 0.5 again.  Except,
it also is implemented within merge() such that a SELECT
won't be issued for an incoming instance with partially
NULL primary key if the flag is False.  [ticket:1680]

diff --git a/CHANGES b/CHANGES
index 0d945b9cbfc774aa7fbe3b91d34c2af8e6e324c8..3f73cfe854debfa59997563613be0abe948be813 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -71,6 +71,12 @@ CHANGES
   - Fixed cascade bug in many-to-one relation() when attribute
     was set to None, introduced in r6711 (cascade deleted
     items into session during add()).
+
+  - The "allow_null_pks" flag is now called "allow_partial_pks",
+    defaults to True, acts like it did in 0.5 again.  Except,
+    it also is implemented within merge() such that a SELECT
+    won't be issued for an incoming instance with partially
+    NULL primary key if the flag is False.  [ticket:1680]
     
 - sql
   - The most common result processors conversion function were

diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py
index cc34c22e23138bf85a70bc94aa24842e1ad0d0ee..96a08ea4ef727aa67e6b50823efe77620726c2f2 100644 (file)
--- a/lib/sqlalchemy/orm/__init__.py
+++ b/lib/sqlalchemy/orm/__init__.py
@@ -626,9 +626,17 @@ def mapper(class_, local_table=None, *args, **params):
         :class:`~sqlalchemy.orm.query.Query`.
 
       allow_null_pks
-        This flag is deprecated - allow_null_pks is now "on" in all cases.
-        Rows which contain NULL for some (but not all) of its primary key
-        columns will be considered to have a valid primary key.
+        This flag is deprecated - this is stated as allow_partial_pks
+        which defaults to True.
+    
+      allow_partial_pks
+        Defaults to True.  Indicates that a composite primary key with
+        some NULL values should be considered as possibly existing
+        within the database.   This affects whether a mapper will assign
+        an incoming row to an existing identity, as well as if 
+        session.merge() will check the database first for a particular
+        primary key value.  A "partial primary key" can occur if one 
+        has mapped to an OUTER JOIN, for example.
 
       batch
         Indicates that save operations of multiple entities can be batched

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index a8c525657f09d67680b13820ce7836bebddf8284..21f32780496d56d20c112a80cc250142d9fb11dc 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -90,6 +90,7 @@ class Mapper(object):
                  concrete=False,
                  with_polymorphic=None,
                  allow_null_pks=None,
+                 allow_partial_pks=True,
                  batch=True,
                  column_prefix=None,
                  include_properties=None,
@@ -139,8 +140,12 @@ class Mapper(object):
 
         if allow_null_pks:
             util.warn_deprecated('the allow_null_pks option to Mapper() is '
-                                'deprecated.  It is now on in all cases.')
-            
+                                'deprecated.  It is now allow_partial_pks=False|True, '
+                                'defaults to True.')
+            allow_partial_pks = allow_null_pks
+        
+        self.allow_partial_pks = allow_partial_pks
+        
         if with_polymorphic == '*':
             self.with_polymorphic = ('*', None)
         elif isinstance(with_polymorphic, (tuple, list)):
@@ -1681,7 +1686,11 @@ class Mapper(object):
         populate_instance = extension.get('populate_instance', None)
         append_result = extension.get('append_result', None)
         populate_existing = context.populate_existing or self.always_refresh
-
+        if self.allow_partial_pks:
+            is_not_primary_key = _none_set.issuperset
+        else:
+            is_not_primary_key = _none_set.issubset
+        
         def _instance(row, result):
             if translate_row:
                 ret = translate_row(self, context, row)
@@ -1740,9 +1749,9 @@ class Mapper(object):
             else:
                 # check for non-NULL values in the primary key columns,
                 # else no entity is returned for the row
-                if _none_set.issuperset(identitykey[1]):
+                if is_not_primary_key(identitykey[1]):
                     return None
-                    
+
                 isnew = True
                 currentload = True
                 loaded_instance = True

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index c7a5a361b97ebbcb02b28ea10d08b233a400ab24..0543cd38dd5b459ed2375c55fe382ba78a74b46e 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1153,8 +1153,10 @@ class Session(object):
             merged_state.key = key
             self._update_impl(merged_state)
             new_instance = True
-
-        elif not _none_set.issuperset(key[1]):
+        
+        elif not _none_set.issubset(key[1]) or \
+                    (mapper.allow_partial_pks and 
+                    not _none_set.issuperset(key[1])):
             merged = self.query(mapper.class_).get(key[1])
         else:
             merged = None

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index 4c174ba24dcb9d6b0bfeb90af188493d951dff23..31939a9627f1ef9cd533db25e01ac3c2232ff676 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -573,6 +573,28 @@ class MapperTest(_fixtures.FixtureTest):
             User(id=9, address_id=5),
             User(id=10, address_id=None)])
 
+    @testing.resolve_artifact_names
+    def test_mapping_to_outerjoin_no_partial_pks(self):
+        """test the allow_partial_pks=False flag."""
+
+
+        mapper(User, users.outerjoin(addresses),
+                allow_partial_pks=False,
+               primary_key=[users.c.id, addresses.c.id],
+               properties=dict(
+            address_id=addresses.c.id))
+
+        session = create_session()
+        l = session.query(User).order_by(User.id, User.address_id).all()
+
+        eq_(l, [
+            User(id=7, address_id=1),
+            User(id=8, address_id=2),
+            User(id=8, address_id=3),
+            User(id=8, address_id=4),
+            User(id=9, address_id=5),
+            None])
+
     @testing.resolve_artifact_names
     def test_custom_join(self):
         """select_from totally replace the FROM parameters."""

diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py
index ca7c7942dae7bc2e2013f7b87d5dc6c7f38ea2e2..f8ee559b631a36f27fb57201c2ae8fb8bbbce5d6 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -1,6 +1,6 @@
 from sqlalchemy.test.testing import assert_raises, assert_raises_message
 import sqlalchemy as sa
-from sqlalchemy import Integer, PickleType
+from sqlalchemy import Integer, PickleType, String
 import operator
 from sqlalchemy.test import testing
 from sqlalchemy.util import OrderedSet
@@ -431,8 +431,6 @@ class MergeTest(_fixtures.FixtureTest):
         )
         assert a2 not in sess2.dirty
         
-        
-        
     @testing.resolve_artifact_names
     def test_many_to_many_cascade(self):
 
@@ -925,5 +923,39 @@ class MutableMergeTest(_base.MappedTest):
         
         
         
+class CompositeNullPksTest(_base.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table("data", metadata, 
+            Column('pk1', String(10), primary_key=True),
+            Column('pk2', String(10), primary_key=True),
+        )
+    
+    @classmethod
+    def setup_classes(cls):
+        class Data(_base.ComparableEntity):
+            pass
+    
+    @testing.resolve_artifact_names
+    def test_merge_allow_partial(self):
+        mapper(Data, data)
+        sess = sessionmaker()()
+        
+        d1 = Data(pk1="someval", pk2=None)
         
+        def go():
+            return sess.merge(d1)
+        self.assert_sql_count(testing.db, go, 1)
+
+    @testing.resolve_artifact_names
+    def test_merge_disallow_partial(self):
+        mapper(Data, data, allow_partial_pks=False)
+        sess = sessionmaker()()
+
+        d1 = Data(pk1="someval", pk2=None)
+
+        def go():
+            return sess.merge(d1)
+        self.assert_sql_count(testing.db, go, 0)
+