From 9f020d3ba342cb777d20f7eed0304e8806d47207 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 22 Feb 2010 23:53:35 +0000 Subject: [PATCH] - 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] --- CHANGES | 6 ++++++ lib/sqlalchemy/orm/__init__.py | 14 ++++++++++--- lib/sqlalchemy/orm/mapper.py | 19 ++++++++++++----- lib/sqlalchemy/orm/session.py | 6 ++++-- test/orm/test_mapper.py | 22 ++++++++++++++++++++ test/orm/test_merge.py | 38 +++++++++++++++++++++++++++++++--- 6 files changed, 92 insertions(+), 13 deletions(-) diff --git a/CHANGES b/CHANGES index 0d945b9cbf..3f73cfe854 100644 --- 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 cc34c22e23..96a08ea4ef 100644 --- 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 a8c525657f..21f3278049 100644 --- 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 c7a5a361b9..0543cd38dd 100644 --- 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 4c174ba24d..31939a9627 100644 --- 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 ca7c7942da..f8ee559b63 100644 --- 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) + -- 2.47.3