Bulk update (#2311)

* refactor: remove EmbedPath type from qsFiltersRoot

* Rename reset items function for reusability

CHANGELOG.md

@@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2269, Allow `limit=0` in the request query to return an empty array - @gautam1168, @laurenceisla
  - #2268, Allow returning XML from single-column queries - @fjf2002
  - #2300, RPC POST for function w/single unnamed XML param #2300 - @fjf2002
+ - #1959, Bulk update with PATCH - @steve-chavez
 
 ### Fixed
 
@@ -45,6 +46,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2277, #2238, #1643, Prevent views from breaking one-to-many/many-to-one embeds when using column or FK as target - @steve-chavez
     + When using a column or FK as target for embedding(`/tbl?select=*,col-or-fk(*)`), only tables are now detected and views are not.
     + You can still use a column or an inferred FK on a view to embed a table(`/view?select=*,col-or-fk(*)`)
+ - #1959, An accidental full table PATCH(without filters) is not possible anymore, it requires filters or a `limit` parameter - @steve-chavez, @laurenceisla
  - #2317, Increase the `db-pool-timeout` to 1 hour to prevent frequent high connection latency - @steve-chavez
 
 ### Changed
@@ -61,6 +63,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2277, Views now are not detected when embedding using the column or FK as target (`/view?select=*,column(*)`) - @steve-chavez
    + This embedding form was easily made ambiguous whenever a new view was added.
    + For migrating, clients must be updated to the embedding form of `/view?select=*,other_view!column(*)`.
+ - #1959, A full table PATCH(without filters) is now restricted, it requires a `limit` parameter - @steve-chavez
+   + A `PATCH /tbl` will now result in 0 rows updated, unless `PATCH /tbl?limit=10&order=<pkcol>` is done
 
 ## [9.0.1] - 2022-06-03
 

src/PostgREST/App.hs

@@ -347,8 +347,12 @@ handleCreate identifier@QualifiedIdentifier{..} context@RequestContext{..} = do
       response HTTP.status201 headers mempty
 
 handleUpdate :: QualifiedIdentifier -> RequestContext -> DbHandler Wai.Response
-handleUpdate identifier context@(RequestContext _ _ ApiRequest{..} _) = do
-  WriteQueryResult{..} <- writeQuery MutationUpdate identifier False mempty context
+handleUpdate identifier context@RequestContext{..} = do
+  let
+    ApiRequest{..} = ctxApiRequest
+    pkCols = maybe mempty tablePKCols $ HM.lookup identifier $ dbTables ctxDbStructure
+
+  WriteQueryResult{..} <- writeQuery MutationUpdate identifier False pkCols context
 
   let
     response = gucResponse resGucStatus resGucHeaders

src/PostgREST/Query/QueryBuilder.hs

@@ -107,25 +107,30 @@ mutateRequestToQuery (Insert mainQi iCols body onConflct putConditions returning
   where
     cols = BS.intercalate ", " $ pgFmtIdent <$> S.toList iCols
 
-mutateRequestToQuery (Update mainQi uCols body logicForest range ordts returnings)
+-- An update without a limit is always filtered with a WHERE
+mutateRequestToQuery (Update mainQi uCols body logicForest pkFlts range ordts returnings)
   | S.null uCols =
     -- if there are no columns we cannot do UPDATE table SET {empty}, it'd be invalid syntax
     -- selecting an empty resultset from mainQi gives us the column names to prevent errors when using &select=
     -- the select has to be based on "returnings" to make computed overloaded functions not throw
     SQL.sql $ "SELECT " <> emptyBodyReturnedColumns <> " FROM " <> fromQi mainQi <> " WHERE false"
 
   | range == allRange =
+    let whereLogic | null logicForest = if null pkFlts then "FALSE" else pgrstUpdateBodyF
+                   | otherwise        = logicForestF in
     "WITH " <> normalizedBody body <> " " <>
     "UPDATE " <> mainTbl <> " SET " <> SQL.sql nonRangeCols <> " " <>
-    "FROM (SELECT * FROM json_populate_recordset (null::" <> mainTbl <> " , " <> SQL.sql selectBody <> " )) _ " <>
-    whereLogic <> " " <>
+    "FROM (SELECT * FROM json_populate_recordset (null::" <> mainTbl <> " , " <> SQL.sql selectBody <> " )) pgrst_update_body " <>
+    "WHERE " <> whereLogic <> " " <>
     SQL.sql (returningF mainQi returnings)
 
   | otherwise =
+    let whereLogic | null logicForest = mempty
+                   | otherwise        = " WHERE " <> logicForestF in
     "WITH " <> normalizedBody body <> ", " <>
     "pgrst_update_body AS (SELECT * FROM json_populate_recordset (null::" <> mainTbl <> " , " <> SQL.sql selectBody <> " ) LIMIT 1), " <>
     "pgrst_affected_rows AS (" <>
-      "SELECT " <> SQL.sql rangeIdF <> " FROM " <> mainTbl <>
+      "SELECT " <> SQL.sql rangeIdF <> " FROM " <> mainTbl <> " " <>
       whereLogic <> " " <>
       orderF mainQi ordts <> " " <>
       limitOffsetF range <>
@@ -136,10 +141,11 @@ mutateRequestToQuery (Update mainQi uCols body logicForest range ordts returning
     SQL.sql (returningF mainQi returnings)
 
   where
-    whereLogic = if null logicForest then mempty else " WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree mainQi <$> logicForest)
     mainTbl = SQL.sql (fromQi mainQi)
+    logicForestF = intercalateSnippet " AND " (pgFmtLogicTree mainQi <$> logicForest)
+    pgrstUpdateBodyF = SQL.sql (BS.intercalate " AND " $ (\x -> pgFmtColumn mainQi x <> " = " <> pgFmtColumn (QualifiedIdentifier mempty "pgrst_update_body") x) <$> pkFlts)
     emptyBodyReturnedColumns = if null returnings then "NULL" else BS.intercalate ", " (pgFmtColumn (QualifiedIdentifier mempty $ qiName mainQi) <$> returnings)
-    nonRangeCols = BS.intercalate ", " (pgFmtIdent <> const " = _." <> pgFmtIdent <$> S.toList uCols)
+    nonRangeCols = BS.intercalate ", " (pgFmtIdent <> const " = pgrst_update_body." <> pgFmtIdent <$> S.toList uCols)
     rangeCols = BS.intercalate ", " ((\col -> pgFmtIdent col <> " = (SELECT " <> pgFmtIdent col <> " FROM pgrst_update_body) ") <$> S.toList uCols)
     (whereRangeIdF, rangeIdF) = mutRangeF mainQi (fst . otTerm <$> ordts)
 

src/PostgREST/Request/DbRequestBuilder.hs

@@ -316,14 +316,14 @@ mutateRequest mutation schema tName ApiRequest{..} pkCols readReq = mapLeft ApiR
   case mutation of
     MutationCreate ->
       Right $ Insert qi iColumns body ((,) <$> iPreferResolution <*> Just confCols) [] returnings
-    MutationUpdate -> Right $ Update qi iColumns body combinedLogic iTopLevelRange rootOrder returnings
+    MutationUpdate -> Right $ Update qi iColumns body combinedLogic pkCols iTopLevelRange rootOrder returnings
     MutationSingleUpsert ->
         if null qsLogic &&
            qsFilterFields == S.fromList pkCols &&
            not (null (S.fromList pkCols)) &&
            all (\case
               Filter _ (OpExpr False (Op OpEqual _)) -> True
-              _                                      -> False) filters
+              _                                      -> False) qsFiltersRoot
           then Right $ Insert qi iColumns body (Just (MergeDuplicates, pkCols)) combinedLogic returnings
         else
           Left InvalidFilters
@@ -336,11 +336,9 @@ mutateRequest mutation schema tName ApiRequest{..} pkCols readReq = mapLeft ApiR
       if iPreferRepresentation == None
         then []
         else returningCols readReq pkCols
-    -- update/delete filters can be only on the root table
-    filters = map snd qsFiltersRoot
     logic = map snd qsLogic
     rootOrder = maybe [] snd $ find (\(x, _) -> null x) qsOrder
-    combinedLogic = foldr addFilterToLogicForest logic filters
+    combinedLogic = foldr addFilterToLogicForest logic qsFiltersRoot
     body = payRaw <$> iPayload -- the body is assumed to be json at this stage(ApiRequest validates)
 
 callRequest :: ProcDescription -> ApiRequest -> ReadRequest -> CallRequest

src/PostgREST/Request/QueryParams.hs

@@ -90,8 +90,8 @@ data QueryParams =
     -- ^ &select parameter used to shape the response
     , qsFilters        :: [(EmbedPath, Filter)]
     -- ^ Filters on the result from e.g. &id=e.10
-    , qsFiltersRoot    :: [(EmbedPath, Filter)]
-    -- ^ Subset of the filters that apply on the root table
+    , qsFiltersRoot    :: [Filter]
+    -- ^ Subset of the filters that apply on the root table. These are used on UPDATE/DELETE.
     , qsFiltersNotRoot :: [(EmbedPath, Filter)]
     -- ^ Subset of the filters that do not apply on the root table
     , qsFilterFields   :: S.Set FieldName
@@ -133,7 +133,7 @@ parse qs =
     <*> pRequestColumns columns
     <*> pRequestSelect select
     <*> pRequestFilter `traverse` filters
-    <*> pRequestFilter `traverse` filtersRoot
+    <*> (fmap snd <$> (pRequestFilter `traverse` filtersRoot))
     <*> pRequestFilter `traverse` filtersNotRoot
     <*> pure (S.fromList (fst <$> filters))
     <*> sequenceA (pRequestOnConflict <$> onConflict)

src/PostgREST/Request/Types.hs

@@ -137,6 +137,7 @@ data MutateQuery
       , updCols   :: S.Set FieldName
       , updBody   :: Maybe LBS.ByteString
       , where_    :: [LogicTree]
+      , pkFilters :: [FieldName]
       , mutRange  :: NonnegRange
       , mutOrder  :: [OrderTerm]
       , returning :: [FieldName]

test/spec/Feature/Query/DeleteSpec.hs

@@ -143,7 +143,7 @@ spec =
             , { "id": 3, "name": "item-3" }
             ]|]
 
-        request methodPost "/rpc/reset_limited_items"
+        request methodPost "/rpc/reset_items_tables"
           [("Prefer", "tx=commit")]
           [json| {"tbl_name": "limited_delete_items"} |]
           `shouldRespondWith` ""
@@ -175,7 +175,7 @@ spec =
             , { "id": 3, "name": "item-3" }
             ]|]
 
-        request methodPost "/rpc/reset_limited_items"
+        request methodPost "/rpc/reset_items_tables"
           [("Prefer", "tx=commit")]
           [json| {"tbl_name": "limited_delete_items"} |]
           `shouldRespondWith` ""
@@ -233,7 +233,7 @@ spec =
             , { "id": 3, "name": "item-3" }
             ]|]
 
-        request methodPost "/rpc/reset_limited_items"
+        request methodPost "/rpc/reset_items_tables"
           [("Prefer", "tx=commit")]
           [json| {"tbl_name": "limited_delete_items_view"} |]
           `shouldRespondWith` ""
@@ -265,7 +265,7 @@ spec =
             , { "id": 3, "name": "item-3" }
             ]|]
 
-        request methodPost "/rpc/reset_limited_items"
+        request methodPost "/rpc/reset_items_tables"
           [("Prefer", "tx=commit")]
           [json| {"tbl_name": "limited_delete_items_cpk_view"} |]
           `shouldRespondWith` ""
@@ -297,7 +297,7 @@ spec =
             , { "id": 3, "name": "item-3" }
             ]|]
 
-        request methodPost "/rpc/reset_limited_items"
+        request methodPost "/rpc/reset_items_tables"
           [("Prefer", "tx=commit")]
           [json| {"tbl_name": "limited_delete_items_no_pk"} |]
           `shouldRespondWith` ""

test/spec/Feature/Query/QueryLimitedSpec.hs

@@ -93,16 +93,13 @@ spec =
               { "id": 8, "name": "HaikuOS" } ]|]
             { matchStatus  = 201 }
 
-      it "doesn't affect updates" $
+      it "doesn't affect updates(2 rows would be modified if it did)" $
         request methodPatch "/employees?select=first_name,last_name,occupation"
             [("Prefer", "return=representation")]
             [json| [{"occupation": "Barista"}] |]
           `shouldRespondWith`
-            [json|[
-                { "first_name": "Frances M.", "last_name": "Roe", "occupation": "Barista" },
-                { "first_name": "Daniel B.", "last_name": "Lyon", "occupation": "Barista" },
-                { "first_name": "Edwin S.", "last_name": "Smith", "occupation": "Barista" } ]|]
-            { matchStatus  = 200 }
+            [json|[]|]
+            { matchStatus  = 404 }
 
       it "doesn't affect deletions" $
         request methodDelete "/employees?select=first_name,last_name"

test/spec/Feature/Query/SingularSpec.hs

@@ -65,7 +65,7 @@ spec =
             }
 
       it "raises an error for multiple rows" $ do
-        request methodPatch "/addresses"
+        request methodPatch "/addresses?limit=4&order=id"
             [("Prefer", "tx=commit"), singular]
             [json| { address: "zzz" } |]
           `shouldRespondWith`
@@ -81,7 +81,7 @@ spec =
             [json|[{"id":1,"address":"address 1"}]|]
 
       it "raises an error for multiple rows with return=rep" $ do
-        request methodPatch "/addresses"
+        request methodPatch "/addresses?limit=4&order=id"
             [("Prefer", "tx=commit"), ("Prefer", "return=representation"), singular]
             [json| { address: "zzz" } |]
           `shouldRespondWith`