From: Mike Bayer Date: Fri, 9 Dec 2011 16:12:41 +0000 (-0500) Subject: - Standalone expressions in polymorphic_on X-Git-Tag: rel_0_7_4~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=33ddb48da48af3425a71543da8bab7911455cfee;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Standalone expressions in polymorphic_on propagate to single-table inheritance subclasses so that they are used in the WHERE /JOIN clause to limit rows to that subclass as is the usual behavior. - make sure implicit map to polymorphic_on expr handles creating a label(). Use an explicit name here as _sa_polymorphic_on makes more sense when poking around in _props. --- diff --git a/CHANGES b/CHANGES index 83626cf38d..0a2029a3db 100644 --- a/CHANGES +++ b/CHANGES @@ -122,6 +122,12 @@ CHANGES a common constructed used here. [ticket:2345] and part of [ticket:2238] + Standalone expressions in polymorphic_on + propagate to single-table inheritance + subclasses so that they are used in the + WHERE /JOIN clause to limit rows to that + subclass as is the usual behavior. + - [feature] IdentitySet supports the - operator as the same as difference(), handy when dealing with Session.dirty etc. [ticket:2301] diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index f8e20c969e..66341ecfb6 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -861,26 +861,14 @@ class Mapper(object): routine will run when an instance is created. """ - # do a special check for the "discriminiator" column, as it - # may only be present in the 'with_polymorphic' selectable - # but we need it for the base mapper setter = False - if self.polymorphic_on is None: - for mapper in self.iterate_to_root(): - # try to set up polymorphic on using - # correesponding_column(); else leave - # as None - if mapper.polymorphic_on is not None: - self.polymorphic_on = \ - self.mapped_table.corresponding_column( - mapper.polymorphic_on) - break - if self.polymorphic_on is not None: setter = True if isinstance(self.polymorphic_on, basestring): + # polymorphic_on specified as as string - link + # it to mapped ColumnProperty try: self.polymorphic_on = self._props[self.polymorphic_on] except KeyError: @@ -890,10 +878,15 @@ class Mapper(object): "mapped to this name." % self.polymorphic_on) if self.polymorphic_on in self._columntoproperty: + # polymorphic_on is a column that is already mapped + # to a ColumnProperty prop = self._columntoproperty[self.polymorphic_on] polymorphic_key = prop.key self.polymorphic_on = prop.columns[0] + polymorphic_key = prop.key elif isinstance(self.polymorphic_on, MapperProperty): + # polymorphic_on is directly a MapperProperty, + # ensure it's a ColumnProperty if not isinstance(self.polymorphic_on, properties.ColumnProperty): raise sa_exc.ArgumentError( "Only direct column-mapped " @@ -903,14 +896,29 @@ class Mapper(object): self.polymorphic_on = prop.columns[0] polymorphic_key = prop.key elif not expression.is_column(self.polymorphic_on): + # polymorphic_on is not a Column and not a ColumnProperty; + # not supported right now. raise sa_exc.ArgumentError( "Only direct column-mapped " "property or SQL expression " "can be passed for polymorphic_on" ) else: + # polymorphic_on is a Column or SQL expression and doesn't + # appear to be mapped. + # this means it can be 1. only present in the with_polymorphic + # selectable or 2. a totally standalone SQL expression which we'd + # hope is compatible with this mapper's mapped_table col = self.mapped_table.corresponding_column(self.polymorphic_on) if col is None: + # polymorphic_on doesn't derive from any column/expression + # isn't present in the mapped table. + # we will make a "hidden" ColumnProperty for it. + # Just check that if it's directly a schema.Column and we + # have with_polymorphic, it's likely a user error if the + # schema.Column isn't represented somehow in either mapped_table or + # with_polymorphic. Otherwise as of 0.7.4 we just go with it + # and assume the user wants it that way (i.e. a CASE statement) setter = False instrument = False col = self.polymorphic_on @@ -924,6 +932,13 @@ class Mapper(object): "loads will not function properly" % col.description) else: + # column/expression that polymorphic_on derives from + # is present in our mapped table + # and is probably mapped, but polymorphic_on itself + # is not. This happens when + # the polymorphic_on is only directly present in the + # with_polymorphic selectable, as when use polymorphic_union. + # we'll make a separate ColumnProperty for it. instrument = True key = getattr(col, 'key', None) @@ -933,7 +948,7 @@ class Mapper(object): "Cannot exclude or override the discriminator column %r" % col.key) else: - col = col.label(None) + self.polymorphic_on = col = col.label("_sa_polymorphic_on") key = col.key self._configure_property( @@ -941,12 +956,36 @@ class Mapper(object): properties.ColumnProperty(col, _instrument=instrument), init=init, setparent=True) polymorphic_key = key + else: + # no polymorphic_on was set. + # check inheriting mappers for one. + for mapper in self.iterate_to_root(): + # determine if polymorphic_on of the parent + # should be propagated here. If the col + # is present in our mapped table, or if our mapped + # table is the same as the parent (i.e. single table + # inheritance), we can use it + if mapper.polymorphic_on is not None: + if self.mapped_table is mapper.mapped_table: + self.polymorphic_on = mapper.polymorphic_on + else: + self.polymorphic_on = \ + self.mapped_table.corresponding_column( + mapper.polymorphic_on) + # we can use the parent mapper's _set_polymorphic_identity + # directly; it ensures the polymorphic_identity of the + # instance's mapper is used so is portable to subclasses. + if self.polymorphic_on is not None: + self._set_polymorphic_identity = mapper._set_polymorphic_identity + else: + self._set_polymorphic_identity = None + return if setter: def _set_polymorphic_identity(state): dict_ = state.dict state.get_impl(polymorphic_key).set(state, dict_, - self.polymorphic_identity, None) + state.manager.mapper.polymorphic_identity, None) self._set_polymorphic_identity = _set_polymorphic_identity else: diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index c9aa5fc9be..dab2d56390 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -186,19 +186,62 @@ class PolymorphicOnNotLocalTest(fixtures.MappedTest): self._roundtrip() - def test_polymorphic_on_expr_implicit_map(self): + def test_polymorphic_on_expr_implicit_map_no_label_joined(self): t2, t1 = self.tables.t2, self.tables.t1 Parent, Child = self.classes.Parent, self.classes.Child expr = case([ (t1.c.x=="p", "parent"), (t1.c.x=="c", "child"), - ],else_ = t1.c.x).label("foo") + ],else_ = t1.c.x) + mapper(Parent, t1, polymorphic_identity="parent", + polymorphic_on=expr) + mapper(Child, t2, inherits=Parent, polymorphic_identity="child") + + self._roundtrip() + + def test_polymorphic_on_expr_implicit_map_w_label_joined(self): + t2, t1 = self.tables.t2, self.tables.t1 + Parent, Child = self.classes.Parent, self.classes.Child + expr = case([ + (t1.c.x=="p", "parent"), + (t1.c.x=="c", "child"), + ],else_ = t1.c.x).label(None) mapper(Parent, t1, polymorphic_identity="parent", polymorphic_on=expr) mapper(Child, t2, inherits=Parent, polymorphic_identity="child") self._roundtrip() + def test_polymorphic_on_expr_implicit_map_no_label_single(self): + """test that single_table_criterion is propagated + with a standalone expr""" + t2, t1 = self.tables.t2, self.tables.t1 + Parent, Child = self.classes.Parent, self.classes.Child + expr = case([ + (t1.c.x=="p", "parent"), + (t1.c.x=="c", "child"), + ],else_ = t1.c.x) + mapper(Parent, t1, polymorphic_identity="parent", + polymorphic_on=expr) + mapper(Child, inherits=Parent, polymorphic_identity="child") + + self._roundtrip() + + def test_polymorphic_on_expr_implicit_map_w_label_single(self): + """test that single_table_criterion is propagated + with a standalone expr""" + t2, t1 = self.tables.t2, self.tables.t1 + Parent, Child = self.classes.Parent, self.classes.Child + expr = case([ + (t1.c.x=="p", "parent"), + (t1.c.x=="c", "child"), + ],else_ = t1.c.x).label(None) + mapper(Parent, t1, polymorphic_identity="parent", + polymorphic_on=expr) + mapper(Child, inherits=Parent, polymorphic_identity="child") + + self._roundtrip() + def test_polymorphic_on_column_prop(self): t2, t1 = self.tables.t2, self.tables.t1 Parent, Child = self.classes.Parent, self.classes.Child @@ -269,6 +312,11 @@ class PolymorphicOnNotLocalTest(fixtures.MappedTest): [Parent, Child, Parent] ) + eq_( + [type(t) for t in s.query(Child).all()], + [Child] + ) + class FalseDiscriminatorTest(fixtures.MappedTest): @classmethod diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index 1aae8215fe..469bb626a5 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -715,17 +715,21 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): 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 + + + # putting the discriminator column in exclude_properties, + # very weird. As of 0.7.4 this re-maps it. class Foo(Person): pass assert_props(Empty, ['empty_id']) - assert_raises( - sa.exc.InvalidRequestError, - mapper, + mapper( Foo, inherits=Person, polymorphic_identity='foo', exclude_properties=('type', ), - ) + ) + assert hasattr(Foo, "type") + assert Foo.type.property.columns[0] is t.c.type + @testing.provide_metadata def test_prop_filters_defaults(self): metadata = self.metadata