From e29a4e92bfc9d089ec231d87ace8929804a64e1c Mon Sep 17 00:00:00 2001
From: steve-chavez <stevechavezast@gmail.com>
Date: Thu, 27 Jul 2023 19:39:48 -0500
Subject: [PATCH] fix: null filtering on embed when column=relation

---
 CHANGELOG.md                                  |  1 +
 src/PostgREST/Plan.hs                         |  8 ++----
 test/spec/Feature/Query/RelatedQueriesSpec.hs | 26 ++++++++++---------
 test/spec/fixtures/schema.sql                 | 21 +++++++++++++++
 4 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ca93c0ca91..2fe8be0aa0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2594, Fix unused index on jsonb/jsonb arrow filter and order (``/bets?data->>contractId=eq.1`` and ``/bets?order=data->>contractId``) - @steve-chavez
  - #2861, Fix character and bit columns with fixed length not inserting/updating properly - @laurenceisla
    + Fixes the error "value too long for type character(1)" when the char length of the column was bigger than one.
+ - #2862, Fix null filtering on embedded resource when using a column name equal to the relation name - @steve-chave
 
 ## [11.1.0] - 2023-06-07
 
diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs
index 18ccb7859c..a3586771a3 100644
--- a/src/PostgREST/Plan.hs
+++ b/src/PostgREST/Plan.hs
@@ -567,6 +567,8 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
 
 -- | Searches for null filters on embeds, e.g. `projects=not.is.null` on `GET /clients?select=*,projects(*)&projects=not.is.null`
 --
+-- (It doesn't err but uses an Either ApiRequestError type so it can combine with the other functions that modify the read plan tree)
+--
 -- Setup:
 --
 -- >>> let nullOp    = OpExpr True (Is TriNull)
@@ -628,11 +630,6 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
 --
 -- >>> ReadPlan.where_ . rootLabel <$> addNullEmbedFilters (readPlanTree nonNullOp subForestPlan)
 -- Right [CoercibleStmnt (CoercibleFilterNullEmbed False "clients_projects_1")]
---
--- It fails if operators other than is.null or not.is.null on the embedding are used.
---
--- >>> ReadPlan.where_ . rootLabel <$> addNullEmbedFilters (readPlanTree notEqOp subForestPlan)
--- Left (UnacceptableFilter "projects")
 addNullEmbedFilters :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
 addNullEmbedFilters (Node rp@ReadPlan{where_=curLogic} forest) = do
   let forestReadPlans = rootLabel <$> forest
@@ -647,7 +644,6 @@ addNullEmbedFilters (Node rp@ReadPlan{where_=curLogic} forest) = do
         let foundRP = find (\ReadPlan{relName, relAlias} -> fld == fromMaybe relName relAlias) rPlans in
         case (foundRP, opExpr) of
           (Just ReadPlan{relAggAlias}, OpExpr b (Is TriNull)) -> Right $ CoercibleStmnt $ CoercibleFilterNullEmbed b relAggAlias
-          (Just ReadPlan{relName}, _)                         -> Left $ UnacceptableFilter relName
           _                                                   -> Right flt
       flt@(CoercibleStmnt _) ->
         Right flt
diff --git a/test/spec/Feature/Query/RelatedQueriesSpec.hs b/test/spec/Feature/Query/RelatedQueriesSpec.hs
index 9763e23f00..0db178cf9d 100644
--- a/test/spec/Feature/Query/RelatedQueriesSpec.hs
+++ b/test/spec/Feature/Query/RelatedQueriesSpec.hs
@@ -233,18 +233,6 @@ spec = describe "related queries" $ do
         , matchHeaders = [matchContentTypeJson]
         }
 
-    it "only works with is null or is not null operators" $
-      get "/projects?select=name,clients(*)&clients=eq.3" `shouldRespondWith`
-        [json|{
-          "code":"PGRST120",
-          "details":"Only is null or not is null filters are allowed on embedded resources",
-          "hint":null,
-          "message":"Bad operator on the 'clients' embedded resource"
-        }|]
-        { matchStatus  = 400
-        , matchHeaders = [matchContentTypeJson]
-        }
-
     it "doesn't interfere filtering when embedding using the column name" $
       get "/projects?select=name,client_id,client:client_id(name)&client_id=eq.2" `shouldRespondWith`
         [json|[
@@ -254,3 +242,17 @@ spec = describe "related queries" $ do
         { matchStatus  = 200
         , matchHeaders = [matchContentTypeJson]
         }
+
+    it "doesn't interfere filtering on column names used for disambiguation" $
+      get "/user_friend?select=*,user1(*)&user1=eq.a02fb934-3a4d-469b-a6b6-4fcd88b973cf" `shouldRespondWith`
+        [json|[]|]
+        { matchStatus  = 200
+        , matchHeaders = [matchContentTypeJson]
+        }
+
+    it "doesn't interfere filtering on column names that are the same as the relation name" $
+      get "/tournaments?select=*,status(*)&status=eq.3" `shouldRespondWith`
+        [json|[]|]
+        { matchStatus  = 200
+        , matchHeaders = [matchContentTypeJson]
+        }
diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql
index 9c5260ef20..8b7315399d 100644
--- a/test/spec/fixtures/schema.sql
+++ b/test/spec/fixtures/schema.sql
@@ -3308,3 +3308,24 @@ CREATE TABLE bitchar_with_length (
   bit bit(5),
   char char(5)
 );
+
+--- https://github.com/PostgREST/postgrest/issues/2862
+create table profiles (
+  id uuid primary key,
+  username text null
+);
+
+create table user_friend (
+  id bigint primary key,
+  user1 uuid references profiles (id),
+  user2 uuid references profiles (id)
+);
+
+create table status(
+  id bigint primary key
+);
+
+create table tournaments(
+  id bigint primary key,
+  status bigint references status(id)
+);