1101 Fix joins with ForeignKey columns with db_column_name specified (#1102)

* fix joins with `db_column_name`

* update tests

Author: dantownsend

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)

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,
                 }
             ],
         )