server: Improve queryModifiesSchemaCache check for run_sql (#4283)

The previous check was too conservative and acquired a lock on the
schema cache in situations where it was unnecessary. This change
exposes the logic run_sql uses to determine whether to use the
metadata check to make the check more precise.

server/src-lib/Hasura/RQL/DDL/Schema.hs

@@ -32,6 +32,7 @@ module Hasura.RQL.DDL.Schema
 
  , RunSQL(..)
  , runRunSQL
+ , isSchemaCacheBuildRequiredRunSQL
  ) where
 
 import           Hasura.Prelude
@@ -86,24 +87,13 @@ instance ToJSON RunSQL where
           Q.ReadWrite -> False
       ]
 
-runRunSQL :: (MonadTx m, CacheRWM m, HasSQLGenCtx m) => RunSQL -> m EncJSON
-runRunSQL RunSQL {..} = do
-  -- see Note [Checking metadata consistency in run_sql]
-  let metadataCheckNeeded = case rTxAccessMode of
+-- | see Note [Checking metadata consistency in run_sql]
+isSchemaCacheBuildRequiredRunSQL :: RunSQL -> Bool
+isSchemaCacheBuildRequiredRunSQL RunSQL {..} =
+  case rTxAccessMode of
     Q.ReadOnly  -> False
     Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency
-  if metadataCheckNeeded
-    then withMetadataCheck rCascade $ execRawSQL rSql
-    else execRawSQL rSql
   where
-    execRawSQL :: (MonadTx m) => Text -> m EncJSON
-    execRawSQL =
-      fmap (encJFromJValue @RunSQLRes) . liftTx . Q.multiQE rawSqlErrHandler . Q.fromText
-      where
-        rawSqlErrHandler txe =
-          (err400 PostgresError "query execution failed") { qeInternal = Just $ toJSON txe }
-
-    -- see Note [Checking metadata consistency in run_sql]
     containsDDLKeyword :: Text -> Bool
     containsDDLKeyword = TDFA.match $$(quoteRegex
       TDFA.defaultCompOpt
@@ -114,6 +104,21 @@ runRunSQL RunSQL {..} = do
         { TDFA.captureGroups = False }
         "\\balter\\b|\\bdrop\\b|\\breplace\\b|\\bcreate function\\b|\\bcomment on\\b")
 
+runRunSQL :: (MonadTx m, CacheRWM m, HasSQLGenCtx m) => RunSQL -> m EncJSON
+runRunSQL q@RunSQL {..}
+  -- see Note [Checking metadata consistency in run_sql]
+  | isSchemaCacheBuildRequiredRunSQL q
+  = withMetadataCheck rCascade $ execRawSQL rSql
+  | otherwise
+  = execRawSQL rSql
+  where
+    execRawSQL :: (MonadTx m) => Text -> m EncJSON
+    execRawSQL =
+      fmap (encJFromJValue @RunSQLRes) . liftTx . Q.multiQE rawSqlErrHandler . Q.fromText
+      where
+        rawSqlErrHandler txe =
+          (err400 PostgresError "query execution failed") { qeInternal = Just $ toJSON txe }
+
 {- Note [Checking metadata consistency in run_sql]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 SQL queries executed by run_sql may change the Postgres schema in arbitrary

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

@@ -1,5 +1,4 @@
 {-# LANGUAGE NamedFieldPuns #-}
-
 module Hasura.Server.Query where
 
 import           Control.Lens
@@ -259,10 +258,7 @@ queryModifiesSchemaCache (RQV1 qi) = case qi of
   RQAddCollectionToAllowlist _    -> True
   RQDropCollectionFromAllowlist _ -> True
 
-  RQRunSql RunSQL{rTxAccessMode, rCheckMetadataConsistency}  ->
-    case rTxAccessMode of
-      Q.ReadOnly  -> False
-      Q.ReadWrite -> fromMaybe True rCheckMetadataConsistency
+  RQRunSql q                      -> isSchemaCacheBuildRequiredRunSQL q
 
   RQReplaceMetadata _             -> True
   RQExportMetadata _              -> False