bulk query should not care about access mode of select or count queries (#3467)

server/src-lib/Hasura/Server/Query.hs

@@ -301,31 +301,40 @@ queryNeedsReload (RQV2 qi) = case qi of
   RQV2SetTableCustomFields _ -> True
   RQV2TrackFunction _        -> True
 
--- TODO: RQSelect query should also be run in READ ONLY mode.
--- But this could be part of console's bulk statement and hence should be added after console changes
 getQueryAccessMode :: (MonadError QErr m) => RQLQuery -> m Q.TxAccess
-getQueryAccessMode (RQV1 q) =
-  case q of
-    RQRunSql RunSQL{rTxAccessMode} -> pure rTxAccessMode
-    RQBulk qs -> assertAllTxAccess (zip [0::Integer ..] qs)
-    _                              -> pure Q.ReadWrite
+getQueryAccessMode q = (fromMaybe Q.ReadOnly) <$> getQueryAccessMode' q
   where
-    assertAllTxAccess = \case
-      [] -> throw400 BadRequest "expected atleast one query in bulk"
-      (_i, q1):[] -> getQueryAccessMode q1
-      q1:q2:qs -> assertSameTxAccess q1 q2 >> assertAllTxAccess (q2:qs)
+    getQueryAccessMode' ::
+         (MonadError QErr m) => RQLQuery -> m (Maybe Q.TxAccess)
+    getQueryAccessMode' (RQV1 q') =
+      case q' of
+        RQSelect _ -> pure Nothing
+        RQCount _ -> pure Nothing
+        RQRunSql RunSQL {rTxAccessMode} -> pure $ Just rTxAccessMode
+        RQBulk qs -> foldM reconcileAccessModeWith Nothing (zip [0 :: Integer ..] qs)
+        _ -> pure $ Just Q.ReadWrite
+      where
+        reconcileAccessModeWith expectedMode (i, query) = do
+          queryMode <- getQueryAccessMode' query
+          onLeft (reconcileAccessModes expectedMode queryMode) $ \errMode ->
+            throw400 BadRequest $
+            "incompatible access mode requirements in bulk query, " <>
+            "expected access mode: " <>
+            (T.pack $ maybe "ANY" show expectedMode) <>
+            " but " <>
+            "$.args[" <>
+            (T.pack $ show i) <>
+            "] forces " <>
+            (T.pack $ show errMode)
+    getQueryAccessMode' (RQV2 _) = pure $ Just Q.ReadWrite
 
-    assertSameTxAccess (i1, q1) (i2, q2) = do
-      accessModeQ1 <- getQueryAccessMode q1
-      accessModeQ2 <- getQueryAccessMode q2
-      if (accessModeQ1 /= accessModeQ2)
-        then
-        throw400 BadRequest $ "incompatible access mode requirements in bulk query: "
-        <> "$.args[" <> (T.pack $ show i1) <> "] requires " <> (T.pack $ show accessModeQ1) <> ", "
-        <> "$.args[" <> (T.pack $ show i2) <> "] requires " <> (T.pack $ show accessModeQ2)
-        else
-        pure accessModeQ1
-getQueryAccessMode (RQV2 _) = pure Q.ReadWrite
+-- | onRight, return reconciled access mode. onLeft, return conflicting access mode
+reconcileAccessModes :: Maybe Q.TxAccess -> Maybe Q.TxAccess -> Either Q.TxAccess (Maybe Q.TxAccess)
+reconcileAccessModes Nothing mode = pure mode
+reconcileAccessModes mode Nothing = pure mode
+reconcileAccessModes (Just mode1) (Just mode2)
+  | mode1 == mode2 = pure $ Just mode1
+  | otherwise = Left mode2
 
 runQueryM
   :: ( QErrM m, CacheRWM m, UserInfoM m, MonadTx m

server/tests-py/queries/v1/bulk/basic.yaml

@@ -2,7 +2,6 @@ description: Bulk query
 url: /v1/query
 status: 200
 response:
-  - message: success
   - affected_rows: 2
   - result_type: TuplesOk
     result:
@@ -12,10 +11,6 @@ response:
 query:
   type: bulk
   args:
-  - type: track_table
-    args:
-      schema: public
-      name: author
   - type: insert
     args:
       table: author

server/tests-py/queries/v1/bulk/mixed_access_mode.yaml

@@ -3,15 +3,11 @@ url: /v1/query
 status: 400
 response:
   path: $
-  error: "incompatible access mode requirements in bulk query: $.args[1] requires READ WRITE, $.args[2] requires READ ONLY"
+  error: "incompatible access mode requirements in bulk query, expected access mode: READ WRITE but $.args[1] forces READ ONLY"
   code: bad-request
 query:
   type: bulk
   args:
-  - type: track_table
-    args:
-      schema: public
-      name: author
   - type: insert
     args:
       table: author

server/tests-py/queries/v1/bulk/select_with_reads.yaml

@@ -0,0 +1,21 @@
+description: Bulk query
+url: /v1/query
+status: 200
+response:
+  - []
+  - result_type: TuplesOk
+    result:
+      - ['count']
+      - ['0']
+query:
+  type: bulk
+  args:
+  - type: select
+    args:
+      table: author
+      columns:
+        - name
+  - type: run_sql
+    args:
+      sql: "select count(*) from author"
+      read_only: true

server/tests-py/queries/v1/bulk/select_with_writes.yaml

@@ -0,0 +1,24 @@
+description: Bulk query
+url: /v1/query
+status: 200
+response:
+  - affected_rows: 2
+  -
+    - name: "author1"
+    - name: "author2"
+query:
+  type: bulk
+  args:
+  - type: insert
+    args:
+      table: author
+      objects:
+        - name: "author1"
+          is_registered: true
+        - name: "author2"
+          is_registered: false
+  - type: select
+    args:
+      table: author
+      columns:
+        - name

server/tests-py/queries/v1/bulk/setup.yaml

@@ -1,8 +1,14 @@
-type: run_sql
+type: bulk
 args:
-  sql: |
-    create table author(
+- type: run_sql
+  args:
+    sql: |
+      create table author(
         id serial primary key,
         name text unique,
         is_registered boolean
-    );
+        );
+- type: track_table
+  args:
+    schema: public
+    name: author

server/tests-py/test_v1_queries.py

@@ -722,3 +722,11 @@ class TestBulkQuery(DefaultTestQueries):
 
     def test_run_bulk_mixed_access_mode(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + '/mixed_access_mode.yaml')
+
+    def test_run_bulk_with_select_and_writes(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/select_with_writes.yaml')
+
+    def test_run_bulk_with_select_and_reads(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/select_with_reads.yaml')
+
+