From db69230fe364141c95be916f2c7e859b353b7124 Mon Sep 17 00:00:00 2001 From: Daniel Townsend Date: Fri, 18 Oct 2024 12:24:27 +0100 Subject: [PATCH] 1101 Fix joins with `ForeignKey` columns with `db_column_name` specified (#1102) * fix joins with `db_column_name` * update tests --- piccolo/query/methods/select.py | 2 +- tests/columns/test_db_column_name.py | 71 +++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/piccolo/query/methods/select.py b/piccolo/query/methods/select.py index 5d7856c5a..c45fdbd46 100644 --- a/piccolo/query/methods/select.py +++ b/piccolo/query/methods/select.py @@ -534,7 +534,7 @@ def _get_joins(self, columns: t.Sequence[Selectable]) -> t.List[str]: _joins.append( f'LEFT JOIN {right_tablename} "{table_alias}"' " ON " - f'({left_tablename}."{key._meta.name}" = "{table_alias}"."{pk_name}")' # noqa: E501 + f'({left_tablename}."{key._meta.db_column_name}" = "{table_alias}"."{pk_name}")' # noqa: E501 ) joins.extend(_joins) diff --git a/tests/columns/test_db_column_name.py b/tests/columns/test_db_column_name.py index 33beffed9..96a182149 100644 --- a/tests/columns/test_db_column_name.py +++ b/tests/columns/test_db_column_name.py @@ -1,12 +1,20 @@ -from piccolo.columns.column_types import Integer, Serial, Varchar -from piccolo.table import Table +import typing as t + +from piccolo.columns.column_types import ForeignKey, Integer, Serial, Varchar +from piccolo.table import Table, create_db_tables_sync, drop_db_tables_sync from tests.base import DBTestCase, engine_is, engines_only, engines_skip +class Manager(Table): + id: Serial + name = Varchar() + + class Band(Table): id: Serial name = Varchar(db_column_name="regrettable_column_name") popularity = Integer() + manager = ForeignKey(Manager, db_column_name="manager_fk") class TestDBColumnName(DBTestCase): @@ -22,10 +30,15 @@ class MyTable(Table): """ def setUp(self): - Band.create_table().run_sync() + create_db_tables_sync(Band, Manager) def tearDown(self): - Band.alter().drop_table().run_sync() + drop_db_tables_sync(Band, Manager) + + def insert_band(self, manager: t.Optional[Manager] = None) -> Band: + band = Band(name="Pythonistas", popularity=1000, manager=manager) + band.save().run_sync() + return band @engines_only("postgres", "cockroach") def test_column_name_correct(self): @@ -45,8 +58,7 @@ def test_save(self): """ Make sure save queries work correctly. """ - band = Band(name="Pythonistas", popularity=1000) - band.save().run_sync() + self.insert_band() band_from_db = Band.objects().first().run_sync() assert band_from_db is not None @@ -56,11 +68,7 @@ def test_create(self): """ Make sure create queries work correctly. """ - band = ( - Band.objects() - .create(name="Pythonistas", popularity=1000) - .run_sync() - ) + band = self.insert_band() self.assertEqual(band.name, "Pythonistas") band_from_db = Band.objects().first().run_sync() @@ -74,7 +82,7 @@ def test_select(self): name to it's alias, but it's hard to predict what behaviour the user wants. """ - Band.objects().create(name="Pythonistas", popularity=1000).run_sync() + self.insert_band() # Make sure we can select all columns bands = Band.select().run_sync() @@ -86,6 +94,7 @@ def test_select(self): "id": bands[0]["id"], "regrettable_column_name": "Pythonistas", "popularity": 1000, + "manager_fk": None, } ], ) @@ -97,6 +106,7 @@ def test_select(self): "id": 1, "regrettable_column_name": "Pythonistas", "popularity": 1000, + "manager_fk": None, } ], ) @@ -123,11 +133,36 @@ def test_select(self): ], ) + def test_join(self): + """ + Make sure that foreign keys with a ``db_column_name`` specified still + work for joins. + + https://github.com/piccolo-orm/piccolo/issues/1101 + + """ + manager = Manager.objects().create(name="Guido").run_sync() + band = self.insert_band(manager=manager) + + bands = Band.select().where(Band.manager.name == "Guido").run_sync() + + self.assertListEqual( + bands, + [ + { + "id": band.id, + "manager_fk": manager.id, + "popularity": 1000, + "regrettable_column_name": "Pythonistas", + } + ], + ) + def test_update(self): """ Make sure update queries work correctly. """ - Band.objects().create(name="Pythonistas", popularity=1000).run_sync() + self.insert_band() Band.update({Band.name: "Pythonistas 2"}, force=True).run_sync() @@ -140,6 +175,7 @@ def test_update(self): "id": bands[0]["id"], "regrettable_column_name": "Pythonistas 2", "popularity": 1000, + "manager_fk": None, } ], ) @@ -151,6 +187,7 @@ def test_update(self): "id": 1, "regrettable_column_name": "Pythonistas 2", "popularity": 1000, + "manager_fk": None, } ], ) @@ -166,6 +203,7 @@ def test_update(self): "id": bands[0]["id"], "regrettable_column_name": "Pythonistas 3", "popularity": 1000, + "manager_fk": None, } ], ) @@ -177,6 +215,7 @@ def test_update(self): "id": 1, "regrettable_column_name": "Pythonistas 3", "popularity": 1000, + "manager_fk": None, } ], ) @@ -199,11 +238,13 @@ def test_delete(self): "id": 1, "regrettable_column_name": "Pythonistas", "popularity": 1000, + "manager_fk": None, }, { "id": 2, "regrettable_column_name": "Rustaceans", "popularity": 500, + "manager_fk": None, }, ], ) @@ -218,6 +259,7 @@ def test_delete(self): "id": 1, "regrettable_column_name": "Pythonistas", "popularity": 1000, + "manager_fk": None, } ], ) @@ -244,11 +286,13 @@ def test_delete_alt(self): "id": result[0]["id"], "regrettable_column_name": "Pythonistas", "popularity": 1000, + "manager_fk": None, }, { "id": result[1]["id"], "regrettable_column_name": "Rustaceans", "popularity": 500, + "manager_fk": None, }, ], ) @@ -263,6 +307,7 @@ def test_delete_alt(self): "id": result[0]["id"], "regrettable_column_name": "Pythonistas", "popularity": 1000, + "manager_fk": None, } ], )