disallow limit/offset on views

CHANGELOG.md

@@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2156, Allow applying `limit/offset` to UPDATE/DELETE to only affect a subset of rows - @steve-chavez
    + Uses the table primary key, so it needs a select privilege on the primary key columns
    + If no primary key is available, it will fallback to using the "ctid" system column(will also require a select privilege on it)
-   + Will work on views if the PK(or "ctid") is present on its SELECT clause
+   + Doesn't work on views and it will throw an error if tried
 
 ### Fixed
 
@@ -50,6 +50,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
    + Previously, those RPCs would return "null" as a body with Content-Type: application/json.
  - #2156, `limit/offset` now limits the affected rows on UPDATE/DELETE  - @steve-chavez
    + Previously, `limit/offset` only limited the returned rows but not the actual updated rows
+ - #2156, using PATCH/DELETE with `limit/offset` throws an error on views - @steve-chavez
  - #2155, `max-rows` is no longer applied on POST/PATCH/PUT/DELETE returned rows - @steve-chavez
    + This was misleading because the affected rows were not really affected by `max-rows`, only the returned rows were limited
 

src/PostgREST/App.hs

@@ -63,7 +63,7 @@ import PostgREST.Config                  (AppConfig (..),
 import PostgREST.Config.PgVersion        (PgVersion (..))
 import PostgREST.ContentType             (ContentType (..))
 import PostgREST.DbStructure             (DbStructure (..),
-                                          findTable,
+                                          findIfView, findTable,
                                           tablePKCols)
 import PostgREST.DbStructure.Identifiers (FieldName,
                                           QualifiedIdentifier (..),
@@ -346,7 +346,10 @@ handleCreate identifier@QualifiedIdentifier{..} context@RequestContext{..} = do
       response HTTP.status201 headers mempty
 
 handleUpdate :: QualifiedIdentifier -> RequestContext -> DbHandler Wai.Response
-handleUpdate identifier context@(RequestContext _ _ ApiRequest{..} _) = do
+handleUpdate identifier context@(RequestContext _ ctxDbStructure ApiRequest{..} _) = do
+  when (iTopLevelRange /= RangeQuery.allRange && findIfView identifier (dbTables ctxDbStructure)) $
+    throwError $ Error.NotImplemented "limit/offset is not implemented for views"
+
   WriteQueryResult{..} <- writeQuery MutationUpdate identifier False mempty context
 
   let
@@ -392,7 +395,10 @@ handleSingleUpsert identifier context@(RequestContext _ _ ApiRequest{..} _) = do
       response HTTP.status204 [] mempty
 
 handleDelete :: QualifiedIdentifier -> RequestContext -> DbHandler Wai.Response
-handleDelete identifier context@(RequestContext _ _ ApiRequest{..} _) = do
+handleDelete identifier context@(RequestContext _ ctxDbStructure ApiRequest{..} _) = do
+  when (iTopLevelRange /= RangeQuery.allRange && findIfView identifier (dbTables ctxDbStructure)) $
+    throwError $ Error.NotImplemented "limit/offset is not implemented for views"
+
   WriteQueryResult{..} <- writeQuery MutationDelete identifier False mempty context
 
   let

src/PostgREST/DbStructure.hs

@@ -23,6 +23,7 @@ module PostgREST.DbStructure
   , queryDbStructure
   , accessibleTables
   , accessibleProcs
+  , findIfView
   , findTable
   , schemaDescription
   , tableCols
@@ -80,7 +81,10 @@ tablePKCols :: DbStructure -> Schema -> TableName -> [Text]
 tablePKCols dbs tSchema tName = pkName <$> filter (\pk -> tSchema == (tableSchema . pkTable) pk && tName == (tableName . pkTable) pk) (dbPrimaryKeys dbs)
 
 findTable :: Schema -> TableName -> [Table] -> Maybe Table
-findTable tSchema tName tbls = find (\tbl -> tableName tbl == tName && tableSchema tbl == tSchema) tbls
+findTable tSchema tName = find (\tbl -> tableSchema tbl == tSchema && tableName tbl == tName)
+
+findIfView :: QualifiedIdentifier -> [Table] -> Bool
+findIfView identifier tbls = maybe False tableIsView (findTable (qiSchema identifier) (qiName identifier) tbls)
 
 -- | The source table column a view column refers to
 type SourceColumn = (Column, ViewColumn)
@@ -138,6 +142,7 @@ decodeTables =
                  <*> column HD.bool
                  <*> column HD.bool
                  <*> column HD.bool
+                 <*> column HD.bool
 
 decodeColumns :: [Table] -> HD.Result [Column]
 decodeColumns tables =
@@ -337,6 +342,7 @@ accessibleTables pgVer =
       n.nspname as table_schema,
       relname as table_name,
       d.description as table_description,
+      c.relkind IN ('v','m') as is_view,
       (
         c.relkind IN ('r','p')
         OR (
@@ -472,6 +478,7 @@ allTables pgVer =
       n.nspname AS table_schema,
       c.relname AS table_name,
       d.description AS table_description,
+      c.relkind IN ('v','m') as is_view,
       (
         c.relkind IN ('r','p')
         OR (

src/PostgREST/DbStructure/Table.hs

@@ -20,6 +20,8 @@ data Table = Table
   { tableSchema      :: Schema
   , tableName        :: TableName
   , tableDescription :: Maybe Text
+    -- TODO Find a better way to separate tables and views
+  , tableIsView      :: Bool
     -- The following fields identify what can be done on the table/view, they're not related to the privileges granted to it
   , tableInsertable  :: Bool
   , tableUpdatable   :: Bool

src/PostgREST/Error.hs

@@ -314,6 +314,7 @@ data Error
   | JwtTokenInvalid Text
   | JwtTokenMissing
   | JwtTokenRequired
+  | NotImplemented Text
   | NoSchemaCacheError
   | NotFound
   | PgErr PgError
@@ -335,6 +336,7 @@ instance PgrstError Error where
   status (PgErr err)             = status err
   status PutMatchingPkError      = HTTP.status400
   status PutRangeNotAllowedError = HTTP.status400
+  status (NotImplemented _)      = HTTP.status501
   status SingularityError{}      = HTTP.status406
   status UnsupportedVerb{}       = HTTP.status405
 
@@ -407,6 +409,12 @@ instance JSON.ToJSON Error where
     "details" .= JSON.Null,
     "hint"    .= JSON.Null]
 
+  toJSON (NotImplemented msg) = JSON.object [
+    "code"    .= GeneralErrorCode07,
+    "message" .= msg,
+    "details" .= JSON.Null,
+    "hint"    .= JSON.Null]
+
   toJSON NotFound = JSON.object []
   toJSON (PgErr err) = JSON.toJSON err
   toJSON (ApiRequestError err) = JSON.toJSON err
@@ -460,6 +468,7 @@ data ErrorCode
   | GeneralErrorCode04
   | GeneralErrorCode05
   | GeneralErrorCode06
+  | GeneralErrorCode07
 
 instance JSON.ToJSON ErrorCode where
   toJSON e = JSON.toJSON (buildErrorCode e)
@@ -504,3 +513,4 @@ buildErrorCode code = "PGRST" <> case code of
   GeneralErrorCode04     -> "504"
   GeneralErrorCode05     -> "505"
   GeneralErrorCode06     -> "506"
+  GeneralErrorCode07     -> "507"

test/spec/Feature/OpenApi/RootSpec.hs

@@ -29,6 +29,7 @@ spec =
         [json| {
             "tableName": "orders_view", "tableSchema": "test",
             "tableDeletable": true, "tableUpdatable": true,
-            "tableInsertable": true, "tableDescription": null
+            "tableIsView":true, "tableInsertable": true,
+            "tableDescription": null
           } |]
         { matchHeaders = [matchContentTypeJson] }

test/spec/Feature/Query/DeleteSpec.hs

@@ -3,7 +3,7 @@ module Feature.Query.DeleteSpec where
 import Network.Wai (Application)
 
 import Network.HTTP.Types
-import Test.Hspec
+import Test.Hspec          hiding (pendingWith)
 import Test.Hspec.Wai
 import Test.Hspec.Wai.JSON
 
@@ -213,7 +213,16 @@ spec =
           `shouldRespondWith` ""
           { matchStatus  = 204 }
 
+      it "doesn't work with views" $
+        request methodDelete "/limited_delete_items_view?limit=1&offset=1"
+            [("Prefer", "tx=commit")]
+            mempty
+          `shouldRespondWith`
+            [json| {"hint":null,"details":null,"code":"PGRST507","message":"limit/offset is not implemented for views"} |]
+            { matchStatus  = 501 }
+
       it "works with views with an inferred pk" $ do
+        pendingWith "not implemented yet"
         get "/limited_delete_items_view"
           `shouldRespondWith`
             [json|[

test/spec/Feature/Query/UpdateSpec.hs

@@ -520,7 +520,16 @@ spec = do
         `shouldRespondWith` ""
         { matchStatus  = 204 }
 
+    it "doesn't work with views" $
+      request methodPatch "/limited_update_items_view?limit=1&offset=1"
+          [("Prefer", "tx=commit")]
+          [json| {"name": "updated-item"} |]
+        `shouldRespondWith`
+          [json| {"hint":null,"details":null,"code":"PGRST507","message":"limit/offset is not implemented for views"} |]
+          { matchStatus  = 501 }
+
     it "works with views with an inferred pk" $ do
+      pendingWith "not implemented yet"
       get "/limited_update_items_view"
         `shouldRespondWith`
           [json|[