diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ec649115..b138a98e6 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/references/api/preferences.rst b/docs/references/api/preferences.rst index 398a49aa6..96dd4f48f 100644 --- a/docs/references/api/preferences.rst +++ b/docs/references/api/preferences.rst @@ -292,3 +292,7 @@ With :ref:`RPC `, 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 ` error. diff --git a/docs/references/errors.rst b/docs/references/errors.rst index 780aa4ad5..0ae8dd2b1 100644 --- a/docs/references/errors.rst +++ b/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**: diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 06f496179..39217610c 100644 --- a/src/PostgREST/Error.hs +++ b/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 diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index fea81c2b3..a5cb25192 100644 --- a/src/PostgREST/Plan.hs +++ b/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 diff --git a/test/spec/Feature/Query/PreferencesSpec.hs b/test/spec/Feature/Query/PreferencesSpec.hs index ea8baaec5..4f2f7b8be 100644 --- a/test/spec/Feature/Query/PreferencesSpec.hs +++ b/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 } diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql index 42c25dcb1..0da32cbe7 100644 --- a/test/spec/fixtures/schema.sql +++ b/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;