From cf7ee67dfea0758717f72165f98a19caefc28272 Mon Sep 17 00:00:00 2001
From: steve-chavez <stevechavezast@gmail.com>
Date: Thu, 28 Sep 2023 19:04:55 -0300
Subject: [PATCH] fix: arrow filter on RPC returning TABLE+composite

---
 CHANGELOG.md                                |  3 ++-
 src/PostgREST/Plan.hs                       | 23 +++++++++------------
 test/spec/Feature/Query/JsonOperatorSpec.hs |  8 +++++++
 test/spec/fixtures/schema.sql               |  9 ++++++++
 4 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 32c8cda596..c5e0585599 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,7 +23,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2939, Fix `count=exact` not being included in `Preference-Applied` - @steve-chavez
  - #2800, Fix not including to-one embed resources that had a `NULL` value in any of the selected fields when doing null filtering on them - @laurenceisla
  - #2846, Fix error when requesting `Prefer: count=<type>` and doing null filtering on embedded resources - @laurenceisla
- - #2846, Fix setting `default_transaction_isolation` unnecessarily - @steve-chavez
+ - #2959, Fix setting `default_transaction_isolation` unnecessarily - @steve-chavez
+ - #2929, Fix arrow filtering on RPC returning dynamic TABLE with composite type - @steve-chavez
 
 ## [11.2.0] - 2023-08-10
 
diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs
index f996c39448..fc5cdc0f93 100644
--- a/src/PostgREST/Plan.hs
+++ b/src/PostgREST/Plan.hs
@@ -245,22 +245,19 @@ resolveTableFieldName table fieldName =
   fromMaybe (unknownField fieldName []) $ HMI.lookup fieldName (tableColumns table) >>=
     Just . resolveColumnField
 
-resolveTableField :: Table -> Field -> CoercibleField
-resolveTableField table (fieldName, []) = resolveTableFieldName table fieldName
-resolveTableField table (fieldName, jp) =
-  case resolveTableFieldName table fieldName of
+-- | Resolve a type within the context based on the given field name and JSON path. Although there are situations where failure to resolve a field is considered an error (see `resolveOrError`), there are also situations where we allow it (RPC calls). If it should be an error and `resolveOrError` doesn't fit, ensure to check the `cfIRType` isn't empty.
+resolveTypeOrUnknown :: ResolverContext -> Field -> CoercibleField
+resolveTypeOrUnknown ResolverContext{..} (fn, jp) =
+  case res of
     -- types that are already json/jsonb don't need to be converted with `to_jsonb` for using arrow operators `data->attr`
     -- this prevents indexes not applying https://github.com/PostgREST/postgrest/issues/2594
-    cf@CoercibleField{cfIRType="json"}  -> cf{cfJsonPath=jp}
-    cf@CoercibleField{cfIRType="jsonb"} -> cf{cfJsonPath=jp}
-    -- other types will get converted `to_jsonb(col)->attr`
+    cf@CoercibleField{cfIRType="json"}  -> cf{cfJsonPath=jp, cfToJson=False}
+    cf@CoercibleField{cfIRType="jsonb"} -> cf{cfJsonPath=jp, cfToJson=False}
+    -- other types will get converted `to_jsonb(col)->attr`, even unknown types
     cf                                  -> cf{cfJsonPath=jp, cfToJson=True}
-
--- | Resolve a type within the context based on the given field name and JSON path. Although there are situations where failure to resolve a field is considered an error (see `resolveOrError`), there are also situations where we allow it (RPC calls). If it should be an error and `resolveOrError` doesn't fit, ensure to check the `cfIRType` isn't empty.
-resolveTypeOrUnknown :: ResolverContext -> Field -> CoercibleField
-resolveTypeOrUnknown ResolverContext{..} field@(fn, jp) =
-  fromMaybe (unknownField fn jp) $ HM.lookup qi tables >>=
-    Just . flip resolveTableField field
+  where
+    res = fromMaybe (unknownField fn jp) $ HM.lookup qi tables >>=
+          Just . flip resolveTableFieldName fn
 
 -- | Install any pre-defined data representation from source to target to coerce this reference.
 --
diff --git a/test/spec/Feature/Query/JsonOperatorSpec.hs b/test/spec/Feature/Query/JsonOperatorSpec.hs
index bff1394d7e..ce1e16c254 100644
--- a/test/spec/Feature/Query/JsonOperatorSpec.hs
+++ b/test/spec/Feature/Query/JsonOperatorSpec.hs
@@ -300,3 +300,11 @@ spec actualPgVersion = describe "json and jsonb operators" $ do
          "code": "PGRST100",
          "hint": null} |]
       { matchStatus = 400, matchHeaders = [matchContentTypeJson] }
+
+  it "works when an RPC returns a dynamic TABLE with a composite type" $
+    get "/rpc/returns_complex?select=val->r&val->i=gt.0.5&order=val->>i.desc" `shouldRespondWith`
+      [json|[
+        {"r":0.3},
+        {"r":0.2}
+      ]|]
+      { matchStatus = 200, matchHeaders = [matchContentTypeJson] }
diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql
index 9a0fa2fd74..f3a392022b 100644
--- a/test/spec/fixtures/schema.sql
+++ b/test/spec/fixtures/schema.sql
@@ -3447,3 +3447,12 @@ create table table_b (
   table_a_id int references table_a(id),
   name text
 );
+
+create or replace function test.returns_complex()
+returns table(id int, val complex) as $$
+  select 1, row(0.1, 0.5)::complex as val
+  union
+  select 2, row(0.2, 0.6)::complex as val
+  union
+  select 3, row(0.3, 0.7)::complex as val;
+$$ language sql;