From: Mike Bayer Date: Wed, 22 Jan 2020 21:27:26 +0000 (-0500) Subject: InstanceState default path is RootRegistry, not tuple X-Git-Tag: rel_1_3_14~37 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3c74e877aa28fc0a840d281f6b3b3d059a42b5bf;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git InstanceState default path is RootRegistry, not tuple Fixed regression caused in 1.3.13 by :ticket:`5056` where a refactor of the ORM path registry system made it such that a path could no longer be compared to an empty tuple, which can occur in a particular kind of joined eager loading path. The "empty tuple" use case has been resolved so that the path registry is compared to a path registry in all cases; the :class:`.PathRegistry` object itself now implements ``__eq__()`` and ``__ne__()`` methods which will take place for all equality comparisons and continue to succeed in the not anticipated case that a non- :class:`.PathRegistry` object is compared, while emitting a warning that this object should not be the subject of the comparison. Fixes: #5110 Change-Id: I6cab6cd771c131d12b17939b369212f12c6bee16 (cherry picked from commit f5eeac3d18892206abcaa30a295d12a799a8fb9b) --- diff --git a/doc/build/changelog/unreleased_13/5110.rst b/doc/build/changelog/unreleased_13/5110.rst new file mode 100644 index 0000000000..54823ab4b6 --- /dev/null +++ b/doc/build/changelog/unreleased_13/5110.rst @@ -0,0 +1,16 @@ +.. change:: + :tags: bug, orm + :tickets: 5110 + + Fixed regression caused in 1.3.13 by :ticket:`5056` where a refactor of the + ORM path registry system made it such that a path could no longer be + compared to an empty tuple, which can occur in a particular kind of joined + eager loading path. The "empty tuple" use case has been resolved so that + the path registry is compared to a path registry in all cases; the + :class:`.PathRegistry` object itself now implements ``__eq__()`` and + ``__ne__()`` methods which will take place for all equality comparisons and + continue to succeed in the not anticipated case that a non- + :class:`.PathRegistry` object is compared, while emitting a warning that + this object should not be the subject of the comparison. + + diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py index 491bf3337b..f4e16a9f27 100644 --- a/lib/sqlalchemy/orm/path_registry.py +++ b/lib/sqlalchemy/orm/path_registry.py @@ -60,7 +60,24 @@ class PathRegistry(object): is_root = False def __eq__(self, other): - return other is not None and self.path == other.path + try: + return other is not None and self.path == other.path + except AttributeError: + util.warn( + "Comparison of PathRegistry to %r is not supported" + % (type(other)) + ) + return False + + def __ne__(self, other): + try: + return other is None or self.path != other.path + except AttributeError: + util.warn( + "Comparison of PathRegistry to %r is not supported" + % (type(other)) + ) + return True def set(self, attributes, key, value): log.debug("set '%s' on path '%s' to '%s'", key, self, value) diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index d85592adf4..4b6d5c53af 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/lib/sqlalchemy/orm/state.py @@ -62,7 +62,7 @@ class InstanceState(interfaces.InspectionAttrInfo): key = None runid = None load_options = util.EMPTY_SET - load_path = () + load_path = PathRegistry.root insert_order = None _strong_obj = None modified = False diff --git a/test/orm/test_loading.py b/test/orm/test_loading.py index d263b12f2c..5e993a27f9 100644 --- a/test/orm/test_loading.py +++ b/test/orm/test_loading.py @@ -1,7 +1,10 @@ from sqlalchemy import exc from sqlalchemy import select +from sqlalchemy import testing from sqlalchemy.orm import aliased from sqlalchemy.orm import loading +from sqlalchemy.orm import mapper +from sqlalchemy.orm import relationship from sqlalchemy.orm import Session from sqlalchemy.testing import mock from sqlalchemy.testing.assertions import assert_raises @@ -10,10 +13,49 @@ from sqlalchemy.testing.assertions import eq_ from sqlalchemy.util import KeyedTuple from . import _fixtures - # class GetFromIdentityTest(_fixtures.FixtureTest): # class LoadOnIdentTest(_fixtures.FixtureTest): -# class InstanceProcessorTest(_fixture.FixtureTest): + + +class InstanceProcessorTest(_fixtures.FixtureTest): + def test_state_no_load_path_comparison(self): + # test issue #5110 + User, Order, Address = self.classes("User", "Order", "Address") + users, orders, addresses = self.tables("users", "orders", "addresses") + + mapper( + User, + users, + properties={ + "addresses": relationship(Address, lazy="joined"), + "orders": relationship( + Order, lazy="joined", order_by=orders.c.id + ), + }, + ) + mapper( + Order, + orders, + properties={"address": relationship(Address, lazy="joined")}, + ) + mapper(Address, addresses) + + s = Session() + + def go(): + eq_( + User( + id=7, + orders=[ + Order(id=1, address=Address(id=1)), + Order(id=3, address=Address(id=1)), + Order(id=5, address=None), + ], + ), + s.query(User).populate_existing().get(7), + ) + + self.assert_sql_count(testing.db, go, 1) class InstancesTest(_fixtures.FixtureTest): diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py index 680862b7b9..bbd28770ca 100644 --- a/test/orm/test_utils.py +++ b/test/orm/test_utils.py @@ -18,6 +18,7 @@ from sqlalchemy.orm.path_registry import RootRegistry from sqlalchemy.testing import assert_raises from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ +from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ from sqlalchemy.util import compat @@ -621,6 +622,31 @@ class PathRegistryTest(_fixtures.FixtureTest): is_(p1 != p4, True) is_(p1 != p5, True) + def test_eq_non_path(self): + umapper = inspect(self.classes.User) + amapper = inspect(self.classes.Address) + u_alias = inspect(aliased(self.classes.User)) + p1 = PathRegistry.coerce((umapper,)) + p2 = PathRegistry.coerce((umapper, umapper.attrs.addresses)) + p3 = PathRegistry.coerce((u_alias, umapper.attrs.addresses)) + p4 = PathRegistry.coerce((u_alias, umapper.attrs.addresses, amapper)) + p5 = PathRegistry.coerce((u_alias,)).token(":*") + + non_object = 54.1432 + + for obj in [p1, p2, p3, p4, p5]: + with expect_warnings( + "Comparison of PathRegistry to " + "<.* 'float'> is not supported" + ): + is_(obj == non_object, False) + + with expect_warnings( + "Comparison of PathRegistry to " + "<.* 'float'> is not supported" + ): + is_(obj != non_object, True) + def test_contains_mapper(self): umapper = inspect(self.classes.User) amapper = inspect(self.classes.Address)