Fix misleading disambiguation error where relationship looks like valid syntax

* Add columns for the m2m relationship

CHANGELOG.md

@@ -30,6 +30,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2042, Keep working when EMFILE(Too many open files) is reached. - @steve-chavez
  - #2147, Ignore `Content-Type` headers for `GET` requests when calling RPCs. Previously, `GET` without parameters, but with `Content-Type: text/plain` or `Content-Type: application/octet-stream` would fail with `404 Not Found`, even if a function without arguments was available.
  - ```
+ - #2155, Ignore `max-rows` on POST, PATCH, PUT and DELETE - @steve-chavez
+ - #2239, Fix misleading disambiguation error where the content of the `relationship` key looks like valid syntax - @laurenceisla
 
 ### Changed
 

src/PostgREST/Error.hs

@@ -126,23 +126,22 @@ instance JSON.ToJSON ApiRequestError where
 compressedRel :: Relationship -> JSON.Value
 compressedRel Relationship{..} =
   let
-    fmtTbl Table{..} = tableSchema <> "." <> tableName
+    fmtEls els = "(" <> T.intercalate ", " els <> ")"
-    fmtEls els = "[" <> T.intercalate ", " els <> "]"
   in
   JSON.object $
     ("embedding" .= (tableName relTable <> " with " <> tableName relForeignTable :: Text))
     : case relCardinality of
         M2M Junction{..} -> [
             "cardinality" .= ("many-to-many" :: Text)
-          , "relationship" .= (fmtTbl junTable <> fmtEls [junConstraint1] <> fmtEls [junConstraint2])
+          , "relationship" .= (tableName junTable <> " using " <> junConstraint1 <> fmtEls (colName <$> junColumns1) <> " and " <> junConstraint2 <> fmtEls (colName <$> junColumns2))
           ]
         M2O cons -> [
             "cardinality" .= ("many-to-one" :: Text)
-          , "relationship" .= (cons <> fmtEls (colName <$> relColumns) <> fmtEls (colName <$> relForeignColumns))
+          , "relationship" .= (cons <> " using " <> tableName relTable <> fmtEls (colName <$> relColumns) <> " and " <> tableName relForeignTable <> fmtEls (colName <$> relForeignColumns))
           ]
         O2M cons -> [
             "cardinality" .= ("one-to-many" :: Text)
-          , "relationship" .= (cons <> fmtEls (colName <$> relColumns) <> fmtEls (colName <$> relForeignColumns))
+          , "relationship" .= (cons <> " using " <> tableName relTable <> fmtEls (colName <$> relColumns) <> " and " <> tableName relForeignTable <> fmtEls (colName <$> relForeignColumns))
           ]
 
 relHint :: [Relationship] -> Text

test/spec/Feature/EmbedDisambiguationSpec.hs

@@ -20,12 +20,12 @@ spec =
               "details": [
                 {
                     "cardinality": "many-to-one",
-                    "relationship": "message_sender_fkey[sender][id]",
+                    "relationship": "message_sender_fkey using message(sender) and person(id)",
                     "embedding": "message with person"
                 },
                 {
                     "cardinality": "many-to-one",
-                    "relationship": "message_sender_fkey[sender][id]",
+                    "relationship": "message_sender_fkey using message(sender) and person_detail(id)",
                     "embedding": "message with person_detail"
                 }
               ],
@@ -37,6 +37,30 @@ spec =
           , matchHeaders = [matchContentTypeJson]
           }
 
+      it "errs when there's a table and view that point to the same fk (composite pk)" $
+        get "/activities?select=fst_shift(*)" `shouldRespondWith`
+          [json|
+            {
+              "details": [
+                {
+                    "cardinality": "one-to-many",
+                    "relationship": "fst_shift using activities(id, schedule_id) and unit_workdays(fst_shift_activity_id, fst_shift_schedule_id)",
+                    "embedding": "activities with unit_workdays"
+                },
+                {
+                    "cardinality": "one-to-many",
+                    "relationship": "fst_shift using activities(id, schedule_id) and unit_workdays_fst_shift(fst_shift_activity_id, fst_shift_schedule_id)",
+                    "embedding": "activities with unit_workdays_fst_shift"
+                }
+              ],
+              "hint": "Try changing 'fst_shift' to one of the following: 'unit_workdays!fst_shift', 'unit_workdays_fst_shift!fst_shift'. Find the desired relationship in the 'details' key.",
+              "message": "Could not embed because more than one relationship was found for 'activities' and 'fst_shift'"
+            }
+          |]
+          { matchStatus  = 300
+          , matchHeaders = [matchContentTypeJson]
+          }
+
       it "errs when there are o2m and m2m cardinalities to the target table" $
         get "/sites?select=*,big_projects(*)" `shouldRespondWith`
           [json|
@@ -44,17 +68,17 @@ spec =
               "details": [
                 {
                   "cardinality": "many-to-one",
-                  "relationship": "main_project[main_project_id][big_project_id]",
+                  "relationship": "main_project using sites(main_project_id) and big_projects(big_project_id)",
                   "embedding": "sites with big_projects"
                 },
                 {
                   "cardinality": "many-to-many",
-                  "relationship": "test.jobs[jobs_site_id_fkey][jobs_big_project_id_fkey]",
+                  "relationship": "jobs using jobs_site_id_fkey(site_id) and jobs_big_project_id_fkey(big_project_id)",
                   "embedding": "sites with big_projects"
                 },
                 {
                   "cardinality": "many-to-many",
-                  "relationship": "test.main_jobs[jobs_site_id_fkey][jobs_big_project_id_fkey]",
+                  "relationship": "main_jobs using jobs_site_id_fkey(site_id) and jobs_big_project_id_fkey(big_project_id)",
                   "embedding": "sites with big_projects"
                 }
               ],
@@ -73,12 +97,12 @@ spec =
               "details": [
                 {
                     "cardinality": "many-to-one",
-                    "relationship": "agents_department_id_fkey[department_id][id]",
+                    "relationship": "agents_department_id_fkey using agents(department_id) and departments(id)",
                     "embedding": "agents with departments"
                 },
                 {
                     "cardinality": "one-to-many",
-                    "relationship": "departments_head_id_fkey[id][head_id]",
+                    "relationship": "departments_head_id_fkey using agents(id) and departments(head_id)",
                     "embedding": "agents with departments"
                 }
               ],
@@ -100,22 +124,22 @@ spec =
               "details": [
                 {
                   "cardinality": "many-to-many",
-                  "relationship": "test.whatev_jobs[whatev_jobs_site_id_1_fkey][whatev_jobs_project_id_1_fkey]",
+                  "relationship": "whatev_jobs using whatev_jobs_site_id_1_fkey(site_id_1) and whatev_jobs_project_id_1_fkey(project_id_1)",
                   "embedding": "whatev_sites with whatev_projects"
                 },
                 {
                   "cardinality": "many-to-many",
-                  "relationship": "test.whatev_jobs[whatev_jobs_site_id_1_fkey][whatev_jobs_project_id_2_fkey]",
+                  "relationship": "whatev_jobs using whatev_jobs_site_id_1_fkey(site_id_1) and whatev_jobs_project_id_2_fkey(project_id_2)",
                   "embedding": "whatev_sites with whatev_projects"
                 },
                 {
                   "cardinality": "many-to-many",
-                  "relationship": "test.whatev_jobs[whatev_jobs_site_id_2_fkey][whatev_jobs_project_id_1_fkey]",
+                  "relationship": "whatev_jobs using whatev_jobs_site_id_2_fkey(site_id_2) and whatev_jobs_project_id_1_fkey(project_id_1)",
                   "embedding": "whatev_sites with whatev_projects"
                 },
                 {
                   "cardinality": "many-to-many",
-                  "relationship": "test.whatev_jobs[whatev_jobs_site_id_2_fkey][whatev_jobs_project_id_2_fkey]",
+                  "relationship": "whatev_jobs using whatev_jobs_site_id_2_fkey(site_id_2) and whatev_jobs_project_id_2_fkey(project_id_2)",
                   "embedding": "whatev_sites with whatev_projects"
                 }
               ],

test/spec/fixtures/privileges.sql

@@ -134,6 +134,7 @@ GRANT ALL ON TABLE
     , schedules
     , activities
     , unit_workdays
+    , unit_workdays_fst_shift
     , stuff
     , loc_test
     , v1.parents

test/spec/fixtures/schema.sql

@@ -2013,6 +2013,12 @@ add constraint fst_shift foreign key           (fst_shift_activity_id, fst_shift
 add constraint snd_shift foreign key           (snd_shift_activity_id, snd_shift_schedule_id)
                          references activities (id, schedule_id);
 
+create view unit_workdays_fst_shift as
+select unit_id, day, fst_shift_activity_id, fst_shift_schedule_id
+from unit_workdays
+where fst_shift_activity_id is not null
+  and fst_shift_schedule_id is not null;
+
 -- for a pre-request function
 create or replace function custom_headers() returns void as $$
 declare