From a65b6b58cd15956f430f89cbbdeb92dffe677628 Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Fri, 3 Apr 2020 12:05:38 +0530 Subject: [PATCH 1/4] run_sql modifies the schema cache if the query is an DDL one * If check_metadata_consistency is set, it will override it --- server/src-lib/Hasura/RQL/DDL/Schema.hs | 23 ++++++++++++----------- server/src-lib/Hasura/Server/Query.hs | 6 +++--- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema.hs b/server/src-lib/Hasura/RQL/DDL/Schema.hs index 886dbcde882cf..8a4806fad3fc3 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema.hs @@ -32,6 +32,7 @@ module Hasura.RQL.DDL.Schema , RunSQL(..) , runRunSQL + , containsDDLKeyword ) where import Hasura.Prelude @@ -86,6 +87,17 @@ instance ToJSON RunSQL where Q.ReadWrite -> False ] +-- see Note [Checking metadata consistency in run_sql] +containsDDLKeyword :: Text -> Bool +containsDDLKeyword = TDFA.match $$(quoteRegex + TDFA.defaultCompOpt + { TDFA.caseSensitive = False + , TDFA.multiline = True + , TDFA.lastStarGreedy = True } + TDFA.defaultExecOpt + { 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 RunSQL {..} = do -- see Note [Checking metadata consistency in run_sql] @@ -103,17 +115,6 @@ runRunSQL RunSQL {..} = do 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 - { TDFA.caseSensitive = False - , TDFA.multiline = True - , TDFA.lastStarGreedy = True } - TDFA.defaultExecOpt - { TDFA.captureGroups = False } - "\\balter\\b|\\bdrop\\b|\\breplace\\b|\\bcreate function\\b|\\bcomment on\\b") - {- Note [Checking metadata consistency in run_sql] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SQL queries executed by run_sql may change the Postgres schema in arbitrary diff --git a/server/src-lib/Hasura/Server/Query.hs b/server/src-lib/Hasura/Server/Query.hs index 78aba43bc5d20..49c0317114df9 100644 --- a/server/src-lib/Hasura/Server/Query.hs +++ b/server/src-lib/Hasura/Server/Query.hs @@ -1,5 +1,5 @@ {-# LANGUAGE NamedFieldPuns #-} - +{-# LANGUAGE RecordWildCards #-} module Hasura.Server.Query where import Control.Lens @@ -259,10 +259,10 @@ queryModifiesSchemaCache (RQV1 qi) = case qi of RQAddCollectionToAllowlist _ -> True RQDropCollectionFromAllowlist _ -> True - RQRunSql RunSQL{rTxAccessMode, rCheckMetadataConsistency} -> + RQRunSql RunSQL{ .. } -> case rTxAccessMode of Q.ReadOnly -> False - Q.ReadWrite -> fromMaybe True rCheckMetadataConsistency + Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency RQReplaceMetadata _ -> True RQExportMetadata _ -> False From d757a3bbe97d023886e8f3458726762f6492d800 Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Wed, 15 Apr 2020 12:00:59 +0530 Subject: [PATCH 2/4] make a common function to determine if a SC build is required --- server/src-lib/Hasura/RQL/DDL/Schema.hs | 14 +++++++++----- server/src-lib/Hasura/Server/Query.hs | 5 +---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema.hs b/server/src-lib/Hasura/RQL/DDL/Schema.hs index 8a4806fad3fc3..e3f9a0b388f3c 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema.hs @@ -32,7 +32,7 @@ module Hasura.RQL.DDL.Schema , RunSQL(..) , runRunSQL - , containsDDLKeyword + , isSchemaCacheBuildRequiredRunSQL ) where import Hasura.Prelude @@ -98,12 +98,16 @@ containsDDLKeyword = TDFA.match $$(quoteRegex { TDFA.captureGroups = False } "\\balter\\b|\\bdrop\\b|\\breplace\\b|\\bcreate function\\b|\\bcomment on\\b") +isSchemaCacheBuildRequiredRunSQL :: RunSQL -> Bool +isSchemaCacheBuildRequiredRunSQL RunSQL {..} = + case rTxAccessMode of + Q.ReadOnly -> False + Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency + runRunSQL :: (MonadTx m, CacheRWM m, HasSQLGenCtx m) => RunSQL -> m EncJSON -runRunSQL RunSQL {..} = do +runRunSQL q@RunSQL {..} = do + let metadataCheckNeeded = isSchemaCacheBuildRequiredRunSQL q -- see Note [Checking metadata consistency in run_sql] - let metadataCheckNeeded = case rTxAccessMode of - Q.ReadOnly -> False - Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency if metadataCheckNeeded then withMetadataCheck rCascade $ execRawSQL rSql else execRawSQL rSql diff --git a/server/src-lib/Hasura/Server/Query.hs b/server/src-lib/Hasura/Server/Query.hs index 49c0317114df9..ad349677e4b95 100644 --- a/server/src-lib/Hasura/Server/Query.hs +++ b/server/src-lib/Hasura/Server/Query.hs @@ -259,10 +259,7 @@ queryModifiesSchemaCache (RQV1 qi) = case qi of RQAddCollectionToAllowlist _ -> True RQDropCollectionFromAllowlist _ -> True - RQRunSql RunSQL{ .. } -> - case rTxAccessMode of - Q.ReadOnly -> False - Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency + RQRunSql q -> isSchemaCacheBuildRequiredRunSQL q RQReplaceMetadata _ -> True RQExportMetadata _ -> False From 3e31b486c7a7b5c0ad006ffb615e82c9edf4614c Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Mon, 20 Apr 2020 13:09:56 +0530 Subject: [PATCH 3/4] remove the unused RecordWildCards pragma from Hasura/Server/Query.hs --- server/src-lib/Hasura/Server/Query.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src-lib/Hasura/Server/Query.hs b/server/src-lib/Hasura/Server/Query.hs index ad349677e4b95..77d6c0dd56c49 100644 --- a/server/src-lib/Hasura/Server/Query.hs +++ b/server/src-lib/Hasura/Server/Query.hs @@ -1,5 +1,4 @@ {-# LANGUAGE NamedFieldPuns #-} -{-# LANGUAGE RecordWildCards #-} module Hasura.Server.Query where import Control.Lens From 2d7bc60db9818c19e980a8f1da8c07f77b293545 Mon Sep 17 00:00:00 2001 From: Alexis King Date: Wed, 22 Apr 2020 15:58:33 -0500 Subject: [PATCH 4/4] style: push auxiliary function into where, remove redundant let --- server/src-lib/Hasura/RQL/DDL/Schema.hs | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema.hs b/server/src-lib/Hasura/RQL/DDL/Schema.hs index e3f9a0b388f3c..0ddd5ea81353b 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema.hs @@ -87,30 +87,30 @@ instance ToJSON RunSQL where Q.ReadWrite -> False ] --- see Note [Checking metadata consistency in run_sql] -containsDDLKeyword :: Text -> Bool -containsDDLKeyword = TDFA.match $$(quoteRegex - TDFA.defaultCompOpt - { TDFA.caseSensitive = False - , TDFA.multiline = True - , TDFA.lastStarGreedy = True } - TDFA.defaultExecOpt - { TDFA.captureGroups = False } - "\\balter\\b|\\bdrop\\b|\\breplace\\b|\\bcreate function\\b|\\bcomment on\\b") - +-- | see Note [Checking metadata consistency in run_sql] isSchemaCacheBuildRequiredRunSQL :: RunSQL -> Bool isSchemaCacheBuildRequiredRunSQL RunSQL {..} = case rTxAccessMode of - Q.ReadOnly -> False + Q.ReadOnly -> False Q.ReadWrite -> fromMaybe (containsDDLKeyword rSql) rCheckMetadataConsistency + where + containsDDLKeyword :: Text -> Bool + containsDDLKeyword = TDFA.match $$(quoteRegex + TDFA.defaultCompOpt + { TDFA.caseSensitive = False + , TDFA.multiline = True + , TDFA.lastStarGreedy = True } + TDFA.defaultExecOpt + { 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 {..} = do - let metadataCheckNeeded = isSchemaCacheBuildRequiredRunSQL q +runRunSQL q@RunSQL {..} -- see Note [Checking metadata consistency in run_sql] - if metadataCheckNeeded - then withMetadataCheck rCascade $ execRawSQL rSql - else execRawSQL rSql + | isSchemaCacheBuildRequiredRunSQL q + = withMetadataCheck rCascade $ execRawSQL rSql + | otherwise + = execRawSQL rSql where execRawSQL :: (MonadTx m) => Text -> m EncJSON execRawSQL =