From: RamonWill <ramonwilliams@hotmail.co.uk>
Date: Tue, 28 Apr 2020 18:19:06 +0000 (-0400)
Subject: fixes #4212 raise an error if selectable and flat/aliased are sent to with_polymorphi... 
X-Git-Tag: rel_1_4_0b1~345^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f554209ebd676fd7c30e4c494a42a049d8afd3f0;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

fixes #4212 raise an error if selectable and flat/aliased are sent to with_polymorphic simultaenously

<!-- Provide a general summary of your proposed changes in the Title field above -->
Hi,
This is a proposed fix for Issue #4212

When the selectable argument is passed to with_polymorphic it is already aliased. Therefore an argument error should be raised if flat is also True. I have added an if statement to the with_polymorphic function, that raises an Argument Error if selectable is none/False AND if flat=True. I have also included a test.

### Checklist
<!-- go over following points. check them with an `x` if they do apply, (they turn into clickable checkboxes once the PR is submitted, so no need to do everything at once)

-->

This pull request is:

- [ ] A documentation / typographical error fix
	- Good to go, no issue or tests are needed
- [X ] A short code fix
	- please include the issue number, and create an issue if none exists, which
	  must include a complete example of the issue.  one line code fixes without an
	  issue and demonstration will not be accepted.
	- Please include: `Fixes: #<issue number>` in the commit message
	- please include tests.   one line code fixes without tests will not be accepted.
- [ ] A new feature implementation
	- please include the issue number, and create an issue if none exists, which must
	  include a complete example of how the feature would look.
	- Please include: `Fixes: #<issue number>` in the commit message
	- please include tests.

Kind Regards,

Ramon

Closes: #5232
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/5232
Pull-request-sha: eae20cdadac82ca1e0352000fe0211e20b590e8b

Change-Id: I49c3022296608ede43de7f508fa993fa97dda81a
---

diff --git a/doc/build/changelog/unreleased_14/4212.rst b/doc/build/changelog/unreleased_14/4212.rst
new file mode 100644
index 0000000000..bdf9a67434
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/4212.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, easy, inheritance, orm
+    :tickets: 4212
+
+    An :class:`.ArgumentError` is now raised if both the selectable and flat
+    parameters are set to True in :func:`.orm.with_polymorphic`.
+    The selectable name is already aliased and applying flat=True
+    overrides the selectable name with an anonymous name that would've
+    previously caused the code to break. Pull request courtesy Ramon Williams.
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 16aa98adb7..28c3114507 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -945,6 +945,13 @@ def with_polymorphic(
        only be specified if querying for one specific subtype only
     """
     primary_mapper = _class_to_mapper(base)
+
+    if selectable not in (None, False) and flat:
+        raise sa_exc.ArgumentError(
+            "the 'flat' and 'selectable' arguments cannot be passed "
+            "simultaneously to with_polymorphic()"
+        )
+
     if _existing_alias:
         assert _existing_alias.mapper is primary_mapper
         classes = util.to_set(classes)
diff --git a/test/orm/inheritance/test_with_poly.py b/test/orm/inheritance/test_with_poly.py
index 3ff6c191c4..dee76fc7b7 100644
--- a/test/orm/inheritance/test_with_poly.py
+++ b/test/orm/inheritance/test_with_poly.py
@@ -1,4 +1,5 @@
 from sqlalchemy import and_
+from sqlalchemy import exc
 from sqlalchemy import or_
 from sqlalchemy import testing
 from sqlalchemy.orm import create_session
@@ -16,6 +17,24 @@ from ._poly_fixtures import Manager
 from ._poly_fixtures import Person
 
 
+class WithPolymorphicAPITest(_Polymorphic, _PolymorphicFixtureBase):
+    def test_no_use_flat_and_aliased(self):
+        sess = create_session()
+
+        subq = sess.query(Person).subquery()
+
+        testing.assert_raises_message(
+            exc.ArgumentError,
+            "the 'flat' and 'selectable' arguments cannot be passed "
+            "simultaneously to with_polymorphic()",
+            with_polymorphic,
+            Person,
+            [Engineer],
+            selectable=subq,
+            flat=True,
+        )
+
+
 class _WithPolymorphicBase(_PolymorphicFixtureBase):
     def test_join_base_to_sub(self):
         sess = create_session()