- the allow_null_pks flag on mapper() is deprecated, and

the feature is turned "on" by default.  This means that
a row which has a non-null value for any of its primary key
columns will be considered an identity.  The need for this
scenario typically only occurs when mapping to an outer join.
[ticket:1339]
- streamlined the NULL check to use set operations

diff --git a/CHANGES b/CHANGES
index 8682440d534e1be0597f7b55d8de8dd4ed79ac14..8f58c1d7379b39c59b9361f7ebbc20f62c441145 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -41,6 +41,13 @@ CHANGES
     deletes the instance_key and removes from any session.)
     [ticket:1052]
 
+  - the allow_null_pks flag on mapper() is deprecated, and 
+    the feature is turned "on" by default.  This means that 
+    a row which has a non-null value for any of its primary key
+    columns will be considered an identity.  The need for this
+    scenario typically only occurs when mapping to an outer join.
+    [ticket:1339]
+    
   - many-to-one "lazyload" fixes:
       - many-to-one relation to a joined-table subclass now
         uses get() for a simple load (known as the "use_get"

diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py
index eeb2e1fd98b2e48037fdc910a08b33695c595ece..6a3c086bf65aac750506133764711b336148d2cd 100644 (file)
--- a/lib/sqlalchemy/orm/__init__.py
+++ b/lib/sqlalchemy/orm/__init__.py
@@ -620,10 +620,9 @@ def mapper(class_, local_table=None, *args, **params):
         :class:`~sqlalchemy.orm.query.Query`.
 
       allow_null_pks
-        Indicates that composite primary keys where one or more (but not all)
-        columns contain NULL is a valid primary key.  Primary keys which
-        contain NULL values usually indicate that a result row does not
-        contain an entity and should be skipped.
+        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.
 
       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 f4cc72cefc654d8d771557797a17997851f03e16..28565bd82fc404fe02d0ce85ec5b5b0daf190565 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -41,6 +41,7 @@ __all__ = (
 _mapper_registry = weakref.WeakKeyDictionary()
 _new_mappers = False
 _already_compiling = False
+_none_set = frozenset([None])
 
 # a list of MapperExtensions that will be installed in all mappers by default
 global_extensions = []
@@ -90,7 +91,7 @@ class Mapper(object):
                  concrete=False,
                  select_table=None,
                  with_polymorphic=None,
-                 allow_null_pks=False,
+                 allow_null_pks=None,
                  batch=True,
                  column_prefix=None,
                  include_properties=None,
@@ -125,7 +126,6 @@ class Mapper(object):
         self.inherit_foreign_keys = inherit_foreign_keys
         self.extension = extension
         self._init_properties = properties or {}
-        self.allow_null_pks = allow_null_pks
         self.delete_orphans = []
         self.batch = batch
         self.eager_defaults = eager_defaults
@@ -137,7 +137,9 @@ class Mapper(object):
         self._requires_row_aliasing = False
         self._inherits_equated_pairs = None
 
-
+        if allow_null_pks:
+            util.warn_deprecated('the allow_null_pks option to Mapper() is deprecated.  It is now on in all cases.')
+            
         self.select_table = select_table
         if select_table:
             util.warn_deprecated('select_table option is deprecated.  Use with_polymorphic=("*", selectable) '
@@ -1657,15 +1659,11 @@ class Mapper(object):
                 if self._should_log_debug:
                     self._log_debug("_instance(): identity key %s not in session" % (identitykey,))
 
-                if self.allow_null_pks:
-                    for x in identitykey[1]:
-                        if x is not None:
-                            break
-                    else:
-                        return None
-                else:
-                    if None in identitykey[1]:
-                        return None
+                # check for non-NULL values in the primary key columns,
+                # else no entity is returned for the row
+                if _none_set.issuperset(identitykey[1]):
+                    return None
+                    
                 isnew = True
                 currentload = True
                 loaded_instance = True

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index 01ee232318a58ee81138c29cef795d548fbf58a6..5d013b65cc0b46d580cdb3349e0a2a43b76ce655 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -533,7 +533,6 @@ class MapperTest(_fixtures.FixtureTest):
 
 
         mapper(User, users.outerjoin(addresses),
-               allow_null_pks=True,
                primary_key=[users.c.id, addresses.c.id],
                properties=dict(
             address_id=addresses.c.id))
@@ -2039,7 +2038,7 @@ class CompositeTypesTest(_base.MappedTest):
             def __init__(self, version):
                 self.version = version
 
-        mapper(Graph, graphs, allow_null_pks=True, properties={
+        mapper(Graph, graphs, properties={
             'version':sa.orm.composite(Version, graphs.c.id,
                                        graphs.c.version_id)})
 

diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py
index 481deb81b1ece7f9be887ffad6f18009201979cd..e542f18856a9560284255fc85b0215601fcabe54 100644 (file)
--- a/test/orm/test_relationships.py
+++ b/test/orm/test_relationships.py
@@ -466,7 +466,7 @@ class RelationTest4(_base.MappedTest):
             pass
         mapper(C, tableC, properties={
             'a':relation(A, cascade="save-update")
-        }, allow_null_pks=True)
+        })
         mapper(A, tableA)
 
         c1 = C()