From: jonathan vanasco Date: Mon, 27 Sep 2021 16:51:32 +0000 (-0400) Subject: Fixes: #4390 X-Git-Tag: rel_2_0_0b1~625^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2432d2ed0b28480c0e1004a47aa74238865105b5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fixes: #4390 Deprecated an undocumented loader option syntax ``".*"``, which appears to be no different than passing a single asterisk, and will emit a deprecation warning if used. This syntax may have been intended for something but there is currently no need for it. The original ticket was to document the `.{WILDCARD}` (e.g. `.*`) format, however this format does not appear to be used or needed by SQLAlchemy and is likely not used by any projects or developers. This PR invokes `util.warn_deprecated` to notify users this functionality is deprecated, and directs them to the #4390 issue if they actually use it. Assuming there are no complaints over this warning in the coming months, this code can be removed in a future major release. Change-Id: I665e3ac26be0a7819246a2ee56fb5a5f32980c91 --- diff --git a/doc/build/changelog/unreleased_14/4390.rst b/doc/build/changelog/unreleased_14/4390.rst new file mode 100644 index 0000000000..abbc664ee8 --- /dev/null +++ b/doc/build/changelog/unreleased_14/4390.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: deprecated, orm + :tickets: 4390 + + Deprecated an undocumented loader option syntax ``".*"``, which appears to + be no different than passing a single asterisk, and will emit a deprecation + warning if used. This syntax may have been intended for something but there + is currently no need for it. + diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index 675c7218bd..30286c1d80 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -919,6 +919,15 @@ class _UnboundLoad(Load): return (_DEFAULT_TOKEN,) # coerce fooload(".*") into "wildcard on default entity" elif key.startswith("." + _WILDCARD_TOKEN): + util.warn_deprecated( + "The undocumented `.{WILDCARD}` format is deprecated " + "and will be removed in a future version as it is " + "believed to be unused. " + "If you have been using this functionality, please " + "comment on Issue #4390 on the SQLAlchemy project " + "tracker.", + version="1.4", + ) key = key[1:] return key.split(".") else: diff --git a/test/orm/test_default_strategies.py b/test/orm/test_default_strategies.py index 9b228bbaa2..9162d63ecd 100644 --- a/test/orm/test_default_strategies.py +++ b/test/orm/test_default_strategies.py @@ -437,7 +437,7 @@ class DefaultStrategyOptionsTest(_fixtures.FixtureTest): def go(): users[:] = ( sess.query(User) - .options(joinedload(".*")) + .options(joinedload("*")) .options(defaultload(User.addresses).joinedload("*")) .options(defaultload(User.orders).joinedload("*")) .options( @@ -548,7 +548,7 @@ class DefaultStrategyOptionsTest(_fixtures.FixtureTest): def go(): users[:] = ( sess.query(User) - .options(subqueryload(".*")) + .options(subqueryload("*")) .options(defaultload(User.addresses).subqueryload("*")) .options(defaultload(User.orders).subqueryload("*")) .options( diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py index bfdfb00b7f..41e9fa4dda 100644 --- a/test/orm/test_deferred.py +++ b/test/orm/test_deferred.py @@ -1563,14 +1563,11 @@ class InheritanceTest(_Polymorphic): def test_defer_on_wildcard_subclass(self): # pretty much the same as load_only except doesn't # exclude the primary key - - # TODO: what is ".*"? this is not documented anywhere, how did this - # get implemented without docs ? see #4390 s = fixture_session() q = ( s.query(Manager) .order_by(Person.person_id) - .options(defer(".*"), undefer(Manager.status)) + .options(defer("*"), undefer(Manager.status)) ) self.assert_compile( q, diff --git a/test/orm/test_deprecations.py b/test/orm/test_deprecations.py index 061cad7e3b..3928b90512 100644 --- a/test/orm/test_deprecations.py +++ b/test/orm/test_deprecations.py @@ -88,8 +88,14 @@ from .inheritance import _poly_fixtures from .inheritance._poly_fixtures import _Polymorphic from .inheritance._poly_fixtures import Company from .inheritance._poly_fixtures import Engineer +from .inheritance._poly_fixtures import Manager +from .inheritance._poly_fixtures import Person from .test_ac_relationships import PartitionByFixture from .test_bind import GetBindTest as _GetBindTest +from .test_default_strategies import ( + DefaultStrategyOptionsTest as _DefaultStrategyOptionsTest, +) +from .test_deferred import InheritanceTest as _deferred_InheritanceTest from .test_dynamic import _DynamicFixture from .test_events import _RemoveListeners from .test_options import PathTest as OptionsPathTest @@ -155,6 +161,13 @@ merge_result_dep = ( r"The merge_result\(\) function is considered legacy as of the 1.x series" ) +dep_exc_wildcard = ( + r"The undocumented `.{WILDCARD}` format is deprecated and will be removed " + r"in a future version as it is believed to be unused. If you have been " + r"using this functionality, please comment on Issue #4390 on the " + r"SQLAlchemy project tracker." +) + def _aliased_join_warning(arg=None): return testing.expect_warnings( @@ -8696,3 +8709,83 @@ class MergeResultTest(_fixtures.FixtureTest): [(x and x.id or None, y and y.id or None) for x, y in it], [(u1.id, u2.id), (u1.id, None), (u2.id, u3.id)], ) + + +class DefaultStrategyOptionsTest(_DefaultStrategyOptionsTest): + def test_joined_path_wildcards(self): + sess = self._upgrade_fixture() + users = [] + + User, Order, Item = self.classes("User", "Order", "Item") + + # test upgrade all to joined: 1 sql + def go(): + users[:] = ( + sess.query(User) + .options(joinedload(".*")) + .options(defaultload(User.addresses).joinedload("*")) + .options(defaultload(User.orders).joinedload("*")) + .options( + defaultload(User.orders) + .defaultload(Order.items) + .joinedload("*") + ) + .order_by(self.classes.User.id) + .all() + ) + + with assertions.expect_deprecated(dep_exc_wildcard): + self.assert_sql_count(testing.db, go, 1) + self._assert_fully_loaded(users) + + def test_subquery_path_wildcards(self): + sess = self._upgrade_fixture() + users = [] + + User, Order = self.classes("User", "Order") + + # test upgrade all to subquery: 1 sql + 4 relationships = 5 + def go(): + users[:] = ( + sess.query(User) + .options(subqueryload(".*")) + .options(defaultload(User.addresses).subqueryload("*")) + .options(defaultload(User.orders).subqueryload("*")) + .options( + defaultload(User.orders) + .defaultload(Order.items) + .subqueryload("*") + ) + .order_by(User.id) + .all() + ) + + with assertions.expect_deprecated(dep_exc_wildcard): + self.assert_sql_count(testing.db, go, 5) + + # verify everything loaded, with no additional sql needed + self._assert_fully_loaded(users) + + +class Deferred_InheritanceTest(_deferred_InheritanceTest): + def test_defer_on_wildcard_subclass(self): + # pretty much the same as load_only except doesn't + # exclude the primary key + + # what is ".*"? this is not documented anywhere, how did this + # get implemented without docs ? see #4390 + s = fixture_session() + with assertions.expect_deprecated(dep_exc_wildcard): + q = ( + s.query(Manager) + .order_by(Person.person_id) + .options(defer(".*"), undefer(Manager.status)) + ) + self.assert_compile( + q, + "SELECT managers.status AS managers_status " + "FROM people JOIN managers ON " + "people.person_id = managers.person_id ORDER BY people.person_id", + ) + # note this doesn't apply to "bound" loaders since they don't seem + # to have this ".*" featue.