Repository: hasura/graphql-engine
File: server/src-lib/Hasura/Server/Query.hs (Haskell)

Code before:
  RQInsert _                      -> False
  RQSelect _                      -> False
  RQUpdate _                      -> False
  RQDelete _                      -> False
  RQCount _                       -> False

  RQAddRemoteSchema _             -> True
  RQRemoveRemoteSchema _          -> True
  RQReloadRemoteSchema _          -> True

  RQCreateEventTrigger _          -> True
  RQDeleteEventTrigger _          -> True
  RQRedeliverEvent _              -> False
  RQInvokeEventTrigger _          -> False

  RQCreateQueryCollection _       -> True
  RQDropQueryCollection _         -> True
  RQAddQueryToCollection _        -> True
  RQDropQueryFromCollection _     -> True
  RQAddCollectionToAllowlist _    -> True
  RQDropCollectionFromAllowlist _ -> True

  RQRunSql RunSQL{ .. }  ->
    case rTxAccessMode of
      Q.ReadOnly  -> False
      Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency

  RQReplaceMetadata _             -> True
  RQExportMetadata _              -> False
  RQClearMetadata _               -> True
  RQReloadMetadata _              -> True

  RQCreateAction _                -> True
  RQDropAction _                  -> True
  RQUpdateAction _                -> True
  RQCreateActionPermission _      -> True
  RQDropActionPermission _        -> True

  RQDumpInternalState _           -> False
  RQSetCustomTypes _              -> True

  RQBulk qs                       -> any queryModifiesSchemaCache qs
queryModifiesSchemaCache (RQV2 qi) = case qi of
  RQV2TrackTable _           -> True
  RQV2SetTableCustomFields _ -> True
  RQV2TrackFunction _        -> True

getQueryAccessMode :: (MonadError QErr m) => RQLQuery -> m Q.TxAccess
getQueryAccessMode q = (fromMaybe Q.ReadOnly) <$> getQueryAccessMode' q
  where
    getQueryAccessMode' ::

Review comment (performance):
It seems like a lot of logic for this is now duplicated between `runRunSQL` and `queryModifiesSchemaCache`, which is mildly unfortunate. It’s also a little sad because it means that `Hasura.Server.Query` has to know about `containsDDLKeyword`, which is really an implementation detail of `Hasura.RQL.DDL.Schema`.

Could we factor out this logic into a common function that encapsulates all the rules about whether or not `run_sql` should rebuild the schema cache, then export that function instead of `containsDDLKeyword`? Then we’d have a single source of truth for the rules, since both `runRunSQL` and `queryModifiesSchemaCache` could use that function to make the choice.

Code after:
  RQSelect _                      -> False
  RQUpdate _                      -> False
  RQDelete _                      -> False
  RQCount _                       -> False

  RQAddRemoteSchema _             -> True
  RQRemoveRemoteSchema _          -> True
  RQReloadRemoteSchema _          -> True

  RQCreateEventTrigger _          -> True
  RQDeleteEventTrigger _          -> True
  RQRedeliverEvent _              -> False
  RQInvokeEventTrigger _          -> False

  RQCreateQueryCollection _       -> True
  RQDropQueryCollection _         -> True
  RQAddQueryToCollection _        -> True
  RQDropQueryFromCollection _     -> True
  RQAddCollectionToAllowlist _    -> True
  RQDropCollectionFromAllowlist _ -> True

  RQRunSql q                      -> isSchemaCacheBuildRequiredRunSQL q

  RQReplaceMetadata _             -> True
  RQExportMetadata _              -> False
  RQClearMetadata _               -> True
  RQReloadMetadata _              -> True

  RQCreateAction _                -> True
  RQDropAction _                  -> True
  RQUpdateAction _                -> True
  RQCreateActionPermission _      -> True
  RQDropActionPermission _        -> True

  RQDumpInternalState _           -> False
  RQSetCustomTypes _              -> True

  RQBulk qs                       -> any queryModifiesSchemaCache qs
queryModifiesSchemaCache (RQV2 qi) = case qi of
  RQV2TrackTable _           -> True
  RQV2SetTableCustomFields _ -> True
  RQV2TrackFunction _        -> True

getQueryAccessMode :: (MonadError QErr m) => RQLQuery -> m Q.TxAccess
getQueryAccessMode q = (fromMaybe Q.ReadOnly) <$> getQueryAccessMode' q
  where
    getQueryAccessMode' ::
         (MonadError QErr m) => RQLQuery -> m (Maybe Q.TxAccess)
    getQueryAccessMode' (RQV1 q') =
      case q' of
        RQSelect _ -> pure Nothing