diff options
| -rw-r--r-- | CHANGES | 27 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/__init__.py | 25 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 58 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/properties.py | 12 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/strategies.py | 19 | ||||
| -rw-r--r-- | test/ext/test_declarative.py | 35 | ||||
| -rw-r--r-- | test/orm/inheritance/test_basic.py | 7 | ||||
| -rw-r--r-- | test/orm/test_mapper.py | 67 |
8 files changed, 197 insertions, 53 deletions
@@ -21,6 +21,33 @@ CHANGES where a one-step Session constructor is desired. Most users should stick with sessionmaker() for general use, however. + + - The include_properties and exclude_properties arguments + to mapper() now accept Column objects as members in + addition to strings. This so that same-named Column + objects, such as those within a join(), can be + disambiguated. + + - A warning is now emitted if a mapper is created against a + join or other single selectable that includes multiple + columns with the same name in its .c. collection, + and those columns aren't explictly named as part of + the same or separate attributes (or excluded). + In 0.7 this warning will be an exception. Note that + this warning is not emitted when the combination occurs + as a result of inheritance, so that attributes + still allow being overridden naturally. + [ticket:1896]. In 0.7 this will be improved further. + + - The primary_key argument to mapper() can now specify + a series of columns that are only a subset of + the calculated "primary key" columns of the mapped + selectable, without an error being raised. This + helps for situations where a selectable's effective + primary key is simpler than the number of columns + in the selectable that are actually marked as + "primary_key", such as a join against two + tables on their primary key columns [ticket:1896]. - An object that's been deleted now gets a flag 'deleted', which prohibits the object from diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 55daeb3db..17d967db4 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -671,22 +671,27 @@ def mapper(class_, local_table=None, *args, **params): :param concrete: If True, indicates this mapper should use concrete table inheritance with its parent mapper. - :param exclude_properties: A list of properties not to map. Columns - present in the mapped table and present in this list will not be - automatically converted into properties. Note that neither this - option nor include_properties will allow an end-run around Python - inheritance. If mapped class ``B`` inherits from mapped class - ``A``, no combination of includes or excludes will allow ``B`` to - have fewer properties than its superclass, ``A``. + :param exclude_properties: A list or set of string column names to + be excluded from mapping. As of SQLAlchemy 0.6.4, this collection + may also include :class:`.Column` objects. Columns named or present + in this list will not be automatically mapped. Note that neither + this option nor include_properties will allow one to circumvent plan + Python inheritance - if mapped class ``B`` inherits from mapped + class ``A``, no combination of includes or excludes will allow ``B`` + to have fewer properties than its superclass, ``A``. :param extension: A :class:`.MapperExtension` instance or list of :class:`~sqlalchemy.orm.interfaces.MapperExtension` instances which will be applied to all operations by this :class:`~sqlalchemy.orm.mapper.Mapper`. - :param include_properties: An inclusive list of properties to map. - Columns present in the mapped table but not present in this list - will not be automatically converted into properties. + :param include_properties: An inclusive list or set of string column + names to map. As of SQLAlchemy 0.6.4, this collection may also + include :class:`.Column` objects in order to disambiguate between + same-named columns in a selectable (such as a + :func:`~.expression.join()`). If this list is not ``None``, columns + present in the mapped table but not named or present in this list + will not be automatically mapped. See also "exclude_properties". :param inherits: Another :class:`~sqlalchemy.orm.Mapper` for which this :class:`~sqlalchemy.orm.Mapper` will have an inheritance diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index c3e4b042e..1f2216cec 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -192,8 +192,14 @@ class Mapper(object): else: self.polymorphic_map = _polymorphic_map - self.include_properties = include_properties - self.exclude_properties = exclude_properties + if include_properties: + self.include_properties = util.to_set(include_properties) + else: + self.include_properties = None + if exclude_properties: + self.exclude_properties = util.to_set(exclude_properties) + else: + self.exclude_properties = None self.compiled = False @@ -471,7 +477,7 @@ class Mapper(object): for col in self._columntoproperty if not hasattr(col, 'table') or col.table not in self._cols_by_table) - + # if explicit PK argument sent, add those columns to the # primary key mappings if self.primary_key_argument: @@ -479,13 +485,14 @@ class Mapper(object): if k.table not in self._pks_by_table: self._pks_by_table[k.table] = util.OrderedSet() self._pks_by_table[k.table].add(k) - - if self.mapped_table not in self._pks_by_table or \ - len(self._pks_by_table[self.mapped_table]) == 0: - raise sa_exc.ArgumentError( - "Mapper %s could not assemble any primary " - "key columns for mapped table '%s'" % - (self, self.mapped_table.description)) + + # otherwise, see that we got a full PK for the mapped table + elif self.mapped_table not in self._pks_by_table or \ + len(self._pks_by_table[self.mapped_table]) == 0: + raise sa_exc.ArgumentError( + "Mapper %s could not assemble any primary " + "key columns for mapped table '%s'" % + (self, self.mapped_table.description)) if self.inherits and \ not self.concrete and \ @@ -537,7 +544,7 @@ class Mapper(object): if self.inherits: for key, prop in self.inherits._props.iteritems(): if key not in self._props and \ - not self._should_exclude(key, key, local=False): + not self._should_exclude(key, key, local=False, column=None): self._adapt_inherited_property(key, prop, False) # create properties for each column in the mapped table, @@ -550,7 +557,8 @@ class Mapper(object): if self._should_exclude( column.key, column_key, - local=self.local_table.c.contains_column(column) + local=self.local_table.c.contains_column(column), + column=column ): continue @@ -583,7 +591,7 @@ class Mapper(object): % col.description) else: instrument = True - if self._should_exclude(col.key, col.key, local=False): + if self._should_exclude(col.key, col.key, local=False, column=col): raise sa_exc.InvalidRequestError( "Cannot exclude or override the discriminator column %r" % col.key) @@ -625,8 +633,18 @@ class Mapper(object): # existing ColumnProperty from an inheriting mapper. # make a copy and append our column to it prop = prop.copy() + else: + util.warn( + "Implicitly combining column %s with column " + "%s under attribute '%s'. This usage will be " + "prohibited in 0.7. Please configure one " + "or more attributes for these same-named columns " + "explicitly." + % (prop.columns[-1], column, key)) + prop.columns.append(column) self._log("appending to existing ColumnProperty %s" % (key)) + elif prop is None or isinstance(prop, ConcreteInheritedProperty): mapped_column = [] for c in columns: @@ -1099,7 +1117,7 @@ class Mapper(object): (MapperProperty, attributes.InstrumentedAttribute)) and \ hasattr(obj, '__get__') - def _should_exclude(self, name, assigned_name, local): + def _should_exclude(self, name, assigned_name, local, column): """determine whether a particular property should be implicitly present on the class. @@ -1121,13 +1139,17 @@ class Mapper(object): getattr(self.class_, assigned_name)): return True - if (self.include_properties is not None and - name not in self.include_properties): + if self.include_properties is not None and \ + name not in self.include_properties and \ + (column is None or column not in self.include_properties): self._log("not including property %s" % (name)) return True - if (self.exclude_properties is not None and - name in self.exclude_properties): + if self.exclude_properties is not None and \ + ( + name in self.exclude_properties or \ + (column is not None and column in self.exclude_properties) + ): self._log("excluding property %s" % (name)) return True diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 7e19d7b16..263b611a5 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -60,7 +60,17 @@ class ColumnProperty(StrategizedProperty): self.__class__.Comparator) self.descriptor = kwargs.pop('descriptor', None) self.extension = kwargs.pop('extension', None) - self.doc = kwargs.pop('doc', getattr(columns[0], 'doc', None)) + + if 'doc' in kwargs: + self.doc = kwargs.pop('doc') + else: + for col in reversed(self.columns): + doc = getattr(col, 'doc', None) + if doc is not None: + self.doc = doc + break + else: + self.doc = None if kwargs: raise TypeError( diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 1c4571aed..4d98e8e62 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -118,17 +118,20 @@ class ColumnLoader(LoaderStrategy): ) def create_row_processor(self, selectcontext, path, mapper, row, adapter): - key, col = self.key, self.columns[0] - if adapter: - col = adapter.columns[col] - - if col is not None and col in row: - def new_execute(state, dict_, row): - dict_[key] = row[col] + key = self.key + # look through list of columns represented here + # to see which, if any, is present in the row. + for col in self.columns: + if adapter: + col = adapter.columns[col] + if col is not None and col in row: + def new_execute(state, dict_, row): + dict_[key] = row[col] + return new_execute, None else: def new_execute(state, dict_, row): state.expire_attribute_pre_commit(dict_, key) - return new_execute, None + return new_execute, None log.class_logger(ColumnLoader) diff --git a/test/ext/test_declarative.py b/test/ext/test_declarative.py index 71e31233b..26e1563fe 100644 --- a/test/ext/test_declarative.py +++ b/test/ext/test_declarative.py @@ -1316,7 +1316,42 @@ class DeclarativeInheritanceTest(DeclarativeTestBase): primary_language = Column('primary_language', String(50)) assert class_mapper(Engineer).inherits is class_mapper(Person) + + @testing.fails_if(lambda: True, "Not implemented until 0.7") + def test_foreign_keys_with_col(self): + """Test that foreign keys that reference a literal 'id' subclass + 'id' attribute behave intuitively. + + See ticket 1892. + + """ + class Booking(Base): + __tablename__ = 'booking' + id = Column(Integer, primary_key=True) + + class PlanBooking(Booking): + __tablename__ = 'plan_booking' + id = Column(Integer, ForeignKey(Booking.id), + primary_key=True) + + # referencing PlanBooking.id gives us the column + # on plan_booking, not booking + class FeatureBooking(Booking): + __tablename__ = 'feature_booking' + id = Column(Integer, ForeignKey(Booking.id), + primary_key=True) + plan_booking_id = Column(Integer, + ForeignKey(PlanBooking.id)) + + plan_booking = relationship(PlanBooking, + backref='feature_bookings') + + assert FeatureBooking.__table__.c.plan_booking_id.\ + references(PlanBooking.__table__.c.id) + assert FeatureBooking.__table__.c.id.\ + references(Booking.__table__.c.id) + def test_with_undefined_foreignkey(self): class Parent(Base): diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index b4aaf13ba..c6dec16b7 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -981,7 +981,8 @@ class OverrideColKeyTest(_base.MappedTest): # s2 gets a new id, base_id is overwritten by the ultimate # PK col assert s2.id == s2.base_id != 15 - + + @testing.emits_warning(r'Implicit') def test_override_implicit(self): # this is how the pattern looks intuitively when # using declarative. @@ -1143,7 +1144,9 @@ class OptimizedLoadTest(_base.MappedTest): # redefine Sub's "id" to favor the "id" col in the subtable. # "id" is also part of the primary join condition - mapper(Sub, sub, inherits=Base, polymorphic_identity='sub', properties={'id':sub.c.id}) + mapper(Sub, sub, inherits=Base, + polymorphic_identity='sub', + properties={'id':[sub.c.id, base.c.id]}) sess = sessionmaker()() s1 = Sub(data='s1data', sub='s1sub') sess.add(s1) diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index e24906a1f..3012f4b43 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -482,18 +482,19 @@ class MapperTest(_fixtures.FixtureTest): class Manager(Employee): pass class Hoho(object): pass class Lala(object): pass - + class Fub(object):pass + class Frob(object):pass class HasDef(object): def name(self): pass p_m = mapper(Person, t, polymorphic_on=t.c.type, include_properties=('id', 'type', 'name')) - e_m = mapper(Employee, inherits=p_m, polymorphic_identity='employee', - properties={ - 'boss': relationship(Manager, backref=backref('peon', ), remote_side=t.c.id) - }, - exclude_properties=('vendor_id',)) + e_m = mapper(Employee, inherits=p_m, + polymorphic_identity='employee', properties={'boss' + : relationship(Manager, backref=backref('peon'), + remote_side=t.c.id)}, + exclude_properties=('vendor_id', )) m_m = mapper(Manager, inherits=e_m, polymorphic_identity='manager', include_properties=('id', 'type')) @@ -506,8 +507,12 @@ class MapperTest(_fixtures.FixtureTest): hd_m = mapper(HasDef, t, column_prefix="h_") + fb_m = mapper(Fub, t, include_properties=(t.c.id, t.c.type)) + frb_m = mapper(Frob, t, column_prefix='f_', + exclude_properties=(t.c.boss_id, + 'employee_number', t.c.vendor_id)) + p_m.compile() - #sa.orm.compile_mappers() def assert_props(cls, want): have = set([n for n in dir(cls) if not n.startswith('_')]) @@ -519,7 +524,8 @@ class MapperTest(_fixtures.FixtureTest): want = set(want) eq_(have, want) - assert_props(HasDef, ['h_boss_id', 'h_employee_number', 'h_id', 'name', 'h_name', 'h_vendor_id', 'h_type']) + assert_props(HasDef, ['h_boss_id', 'h_employee_number', 'h_id', + 'name', 'h_name', 'h_vendor_id', 'h_type']) assert_props(Person, ['id', 'name', 'type']) assert_instrumented(Person, ['id', 'name', 'type']) assert_props(Employee, ['boss', 'boss_id', 'employee_number', @@ -535,24 +541,57 @@ class MapperTest(_fixtures.FixtureTest): assert_props(Vendor, ['vendor_id', 'id', 'name', 'type']) assert_props(Hoho, ['id', 'name', 'type']) assert_props(Lala, ['p_employee_number', 'p_id', 'p_name', 'p_type']) - + assert_props(Fub, ['id', 'type']) + assert_props(Frob, ['f_id', 'f_type', 'f_name', ]) # excluding the discriminator column is currently not allowed class Foo(Person): pass - assert_raises(sa.exc.InvalidRequestError, mapper, Foo, inherits=Person, polymorphic_identity='foo', exclude_properties=('type',) ) + + assert_raises( + sa.exc.InvalidRequestError, + mapper, + Foo, inherits=Person, polymorphic_identity='foo', + exclude_properties=('type', ), + ) @testing.resolve_artifact_names - def test_mapping_to_join(self): - """Mapping to a join""" + def test_mapping_to_join_raises(self): + """Test implicit merging of two cols warns.""" + usersaddresses = sa.join(users, addresses, users.c.id == addresses.c.user_id) - mapper(User, usersaddresses, primary_key=[users.c.id]) + assert_raises_message( + sa.exc.SAWarning, + "Implicitly", + mapper, User, usersaddresses, primary_key=[users.c.id] + ) + sa.orm.clear_mappers() + + @testing.emits_warning(r'Implicitly') + def go(): + # but it works despite the warning + mapper(User, usersaddresses, primary_key=[users.c.id]) + l = create_session().query(User).order_by(users.c.id).all() + eq_(l, self.static.user_result[:3]) + go() + + @testing.resolve_artifact_names + def test_mapping_to_join(self): + """Mapping to a join""" + + usersaddresses = sa.join(users, addresses, users.c.id + == addresses.c.user_id) + mapper(User, usersaddresses, primary_key=[users.c.id], + exclude_properties=[addresses.c.id]) l = create_session().query(User).order_by(users.c.id).all() eq_(l, self.static.user_result[:3]) @testing.resolve_artifact_names def test_mapping_to_join_no_pk(self): - m = mapper(Address, addresses.join(email_bounces)) + m = mapper(Address, + addresses.join(email_bounces), + properties={'id':[addresses.c.id, email_bounces.c.id]} + ) m.compile() assert addresses in m._pks_by_table assert email_bounces not in m._pks_by_table |