fix: max-affected preference not failing for rpc with strict handling

CHANGELOG.md

@@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
   + The exposed schemas are now listed in the `hint` instead of the `message` field.
 - Improve error details of `PGRST301` error by @taimoorzaeem in #4051
 
+### Fixed
+
+- Fix `max-affected` preference not failing with RPC when `handling=strict` by @taimoorzaeem in #4100
+
 ## [13.0.2] - 2025-06-02
 
 ### Fixed

docs/references/api/preferences.rst

@@ -292,3 +292,7 @@ With :ref:`RPC <functions>`, the preference is honored completely on the basis o
       "details": "The query affects 14 rows",
       "hint": null
   }
+
+.. note::
+
+  It is important for functions to return ``SETOF`` or ``TABLE`` when called with ``max-affected`` preference. A violation of this would cause a :ref:`PGRST128 <pgrst128>` error.

docs/references/errors.rst

@@ -267,6 +267,10 @@ Related to the HTTP request elements.
 |               |             | implemented.                                                |
 | PGRST127      |             |                                                             |
 +---------------+-------------+-------------------------------------------------------------+
+| .. _pgrst128: | 400         | ``max-affected`` preference is violated with ``RPC`` call.  |
+|               |             | See :ref:`prefer_max_affected`.                             |
+| PGRST128      |             |                                                             |
++---------------+-------------+-------------------------------------------------------------+
 
 
 .. _pgrst2**:

src/PostgREST/Error.hs

@@ -98,6 +98,7 @@ data ApiRequestError
   | MaxAffectedViolationError Integer
   | InvalidResourcePath
   | OpenAPIDisabled
+  | MaxAffectedRpcViolation
   deriving Show
 
 data QPError = QPError Text Text
@@ -140,6 +141,7 @@ instance PgrstError ApiRequestError where
   status MaxAffectedViolationError{} = HTTP.status400
   status InvalidResourcePath         = HTTP.status404
   status OpenAPIDisabled             = HTTP.status404
+  status MaxAffectedRpcViolation     = HTTP.status400
 
   headers _ = mempty
 
@@ -186,6 +188,7 @@ instance ErrorBody ApiRequestError where
   code InvalidResourcePath         = "PGRST125"
   code OpenAPIDisabled             = "PGRST126"
   code NotImplemented{}            = "PGRST127"
+  code MaxAffectedRpcViolation     = "PGRST128"
 
   -- MESSAGE: Text
   message (QueryParamError (QPError msg _)) = msg
@@ -211,6 +214,7 @@ instance ErrorBody ApiRequestError where
   message InvalidResourcePath            = "Invalid path specified in request URL"
   message OpenAPIDisabled                = "Root endpoint metadata is disabled"
   message (NotImplemented _)             = "Feature not implemented"
+  message MaxAffectedRpcViolation        = "Function must return SETOF or TABLE when max-affected preference is used with handling=strict"
 
   -- DETAILS: Maybe JSON.Value
   details (QueryParamError (QPError _ dets)) = Just $ JSON.String dets

src/PostgREST/Plan.hs

@@ -72,7 +72,8 @@ import PostgREST.SchemaCache.Routine         (MediaHandler (..),
                                               RoutineParam (..),
                                               funcReturnsCompositeAlias,
                                               funcReturnsScalar,
-                                              funcReturnsSetOfScalar)
+                                              funcReturnsSetOfScalar,
+                                              funcReturnsSingle)
 import PostgREST.SchemaCache.Table           (Column (..), Table (..),
                                               TablesMap,
                                               tableColumnsList,
@@ -172,7 +173,7 @@ mutateReadPlan  mutation apiRequest@ApiRequest{iPreferences=Preferences{..},..}
   return $ MutateReadPlan rPlan mPlan SQL.Write handler mediaType mutation qi
 
 callReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> InvokeMethod -> Either Error CallReadPlan
-callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{preferHandling, invalidPrefs},..} invMethod = do
+callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{preferHandling, invalidPrefs, preferMaxAffected},..} invMethod = do
   let paramKeys = case invMethod of
         InvRead _ -> S.fromList $ fst <$> qsParams'
         Inv       -> iColumns
@@ -192,10 +193,15 @@ callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferenc
       cPlan = callPlan proc apiRequest paramKeys args rPlan
   (handler, mediaType)  <- mapLeft ApiRequestError $ negotiateContent conf apiRequest relIdentifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
   if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
+  failMaxAffectedRpcReturnsSingle (preferMaxAffected, preferHandling) proc
   return $ CallReadPlan rPlan cPlan txMode proc handler mediaType invMethod identifier
   where
     qsParams' = QueryParams.qsParams iQueryParams
 
+    failMaxAffectedRpcReturnsSingle :: (Maybe PreferMaxAffected, Maybe PreferHandling) -> Routine -> Either Error ()
+    failMaxAffectedRpcReturnsSingle (Just (PreferMaxAffected _), Just Strict) rout = if funcReturnsSingle rout then Left $ ApiRequestError MaxAffectedRpcViolation else Right ()
+    failMaxAffectedRpcReturnsSingle _ _ = Right ()
+
 hasDefaultSelect :: ReadPlanTree -> Bool
 hasDefaultSelect (Node ReadPlan{select=[CoercibleSelectField{csField=CoercibleField{cfName}}]} []) = cfName == "*"
 hasDefaultSelect _ = False

test/spec/Feature/Query/PreferencesSpec.hs

@@ -192,16 +192,25 @@ spec =
           { matchStatus = 204
           , matchHeaders = ["Preference-Applied" <:> "handling=lenient"]}
 
-      it "should fail with rpc when deleting rows more than prefered" $
-        request methodPost "/rpc/delete_all_items"
+    context "test Prefer: max-affected with rpc" $ do
+      it "should fail with rpc when deleting rows more than prefered with returns setof" $
+        request methodPost "/rpc/delete_items_returns_setof"
           [("Prefer", "handling=strict, max-affected=10")]
           ""
           `shouldRespondWith`
           [json| {"code":"PGRST124","details":"The query affects 15 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"} |]
           { matchStatus = 400 }
 
-      it "should succeed with rpc deleting rows less than prefered" $
-        request methodPost "/rpc/delete_all_items"
+      it "should fail with rpc when deleting rows more than prefered with returns table" $
+        request methodPost "/rpc/delete_items_returns_table"
+          [("Prefer", "handling=strict, max-affected=10")]
+          ""
+          `shouldRespondWith`
+          [json| {"code":"PGRST124","details":"The query affects 15 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"} |]
+          { matchStatus = 400 }
+
+      it "should succeed with rpc deleting rows less than prefered with returns setof" $
+        request methodPost "/rpc/delete_items_returns_setof"
           [("Prefer", "handling=strict, max-affected=20")]
           ""
           `shouldRespondWith`
@@ -209,3 +218,21 @@ spec =
                  {"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},
                  {"id":14},{"id":15}]|]
           { matchStatus = 200 }
+
+      it "should succeed with rpc deleting rows less than prefered with returns table" $
+        request methodPost "/rpc/delete_items_returns_table"
+          [("Prefer", "handling=strict, max-affected=20")]
+          ""
+          `shouldRespondWith`
+          [json|[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":6},{"id":7},
+                 {"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},
+                 {"id":14},{"id":15}]|]
+          { matchStatus = 200 }
+
+      it "should fail with rpc when returns void with handling=strict" $
+        request methodPost "/rpc/delete_items_returns_void"
+          [("Prefer", "handling=strict, max-affected=20")]
+          ""
+          `shouldRespondWith`
+          [json| {"code":"PGRST128","details":null,"hint":null,"message":"Function must return SETOF or TABLE when max-affected preference is used with handling=strict"} |]
+          { matchStatus = 400 }

test/spec/fixtures/schema.sql

@@ -3817,6 +3817,14 @@ where oid = 'test.collision_test_func'::regproc::oid;
 comment on function test.collision_test_func(id integer) is 'fizzbuzz';
 
 
-create or replace function test.delete_all_items() returns setof items as $$
+create or replace function test.delete_items_returns_setof() returns setof items as $$
   delete from items where id <= 15 returning *; -- deletes 15 items, then return them
 $$ language sql;
+
+create or replace function test.delete_items_returns_table() returns table(id bigint) as $$
+  delete from items where id <= 15 returning *;
+$$ language sql;
+
+create or replace function test.delete_items_returns_void() returns void as $$
+  delete from items;
+$$ language sql;