From 0411ca99de1000b3dda983e68500bae754122851 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Mon, 14 Jan 2019 20:36:04 +0530 Subject: [PATCH 1/6] optimise run_sql query, close #1362 --- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 41 +++++++++++++------ server/src-lib/Hasura/Server/Utils.hs | 15 +++++++ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index 4c2d27fee8d65..c49c0f0e4bc2a 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -12,6 +12,7 @@ import Hasura.RQL.DDL.Schema.Diff import Hasura.RQL.DDL.Subscribe import Hasura.RQL.DDL.Utils import Hasura.RQL.Types +import Hasura.Server.Utils (matchRegex) import Hasura.SQL.Types import qualified Database.PG.Query as Q @@ -375,8 +376,9 @@ buildSchemaCache = do data RunSQL = RunSQL - { rSql :: T.Text - , rCascade :: !(Maybe Bool) + { rSql :: T.Text + , rCascade :: !(Maybe Bool) + , rEnforceStateCheck :: !(Maybe Bool) } deriving (Show, Eq, Lift) $(deriveJSON (aesonDrop 1 snakeCase){omitNothingFields=True} ''RunSQL) @@ -389,10 +391,18 @@ data RunSQLRes $(deriveJSON (aesonDrop 2 snakeCase){omitNothingFields=True} ''RunSQLRes) +execRawSQL :: (MonadTx m) => T.Text -> m RunSQLRes +execRawSQL = + liftTx . Q.multiQE rawSqlErrHandler . Q.fromText + where + rawSqlErrHandler txe = + let e = err400 PostgresError "query execution failed" + in e {qeInternal = Just $ toJSON txe} + runSqlP2 :: (QErrM m, CacheRWM m, MonadTx m, MonadIO m, HasHttpManager m) - => RunSQL -> m RespBody -runSqlP2 (RunSQL t cascade) = do + => RunSQL -> m RunSQLRes +runSqlP2 (RunSQL t cascade _) = do -- Drop hdb_views so no interference is caused to the sql query liftTx $ Q.catchE defaultTxErrorHandler clearHdbViews @@ -401,7 +411,7 @@ runSqlP2 (RunSQL t cascade) = do oldMetaU <- liftTx $ Q.catchE defaultTxErrorHandler fetchTableMeta -- Run the SQL - res <- liftTx $ Q.multiQE rawSqlErrHandler $ Q.fromText t + res <- execRawSQL t -- Get the metadata after the sql query newMeta <- liftTx $ Q.catchE defaultTxErrorHandler fetchTableMeta @@ -440,19 +450,26 @@ runSqlP2 (RunSQL t cascade) = do -- refresh the gCtxMap in schema cache refreshGCtxMapInSchema - return $ encode (res :: RunSQLRes) + return res +isAltrOrDrop :: QErrM m => T.Text -> m Bool +isAltrOrDrop = either throwErr return . isMatchedE where - rawSqlErrHandler :: Q.PGTxErr -> QErr - rawSqlErrHandler txe = - let e = err400 PostgresError "query execution failed" - in e {qeInternal = Just $ toJSON txe} + throwErr s = throw500 $ "compiling regex failed: " <> T.pack s + isMatchedE = matchRegex regex False + regex = "alter table|drop table|alter view|drop view|replace view" runRunSQL :: (QErrM m, UserInfoM m, CacheRWM m, MonadTx m, MonadIO m, HasHttpManager m) => RunSQL -> m RespBody -runRunSQL q = - adminOnly >> runSqlP2 q +runRunSQL q@(RunSQL t _ mEnfStChk) = do + adminOnly + encode <$> bool withoutStChk runP2 enfStChk + where + enfStChk = fromMaybe False mEnfStChk + withoutStChk = + bool (execRawSQL t) runP2 =<< isAltrOrDrop t + runP2 = runSqlP2 q -- Should be used only after checking the status resToCSV :: PQ.Result -> ExceptT T.Text IO [[T.Text]] diff --git a/server/src-lib/Hasura/Server/Utils.hs b/server/src-lib/Hasura/Server/Utils.hs index 3302f5c6a881f..25ead7a2bb7d2 100644 --- a/server/src-lib/Hasura/Server/Utils.hs +++ b/server/src-lib/Hasura/Server/Utils.hs @@ -15,6 +15,8 @@ import qualified Data.Text.Encoding.Error as TE import qualified Data.Text.IO as TI import qualified Language.Haskell.TH.Syntax as TH import qualified Text.Ginger as TG +import qualified Text.Regex.TDFA as TDFA +import qualified Text.Regex.TDFA.String as TDFA import Hasura.Prelude @@ -111,3 +113,16 @@ _2 (_, y, _) = y _3 :: (a, b, c) -> c _3 (_, _, z) = z + +-- regex related +matchRegex :: String -> Bool -> T.Text -> Either String Bool +matchRegex regex caseSensitive src = + fmap (`TDFA.match` T.unpack src) compiledRegexE + where + compOpt = TDFA.defaultCompOpt + { TDFA.caseSensitive = caseSensitive + , TDFA.multiline = True + , TDFA.lastStarGreedy = True + } + execOption = TDFA.defaultExecOpt {TDFA.captureGroups = False} + compiledRegexE = TDFA.compile compOpt execOption regex From 8bd39ac344419b4ee6b3f16baaa8a4a3d1215cd1 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Fri, 18 Jan 2019 10:50:38 +0530 Subject: [PATCH 2/6] minor refactor --- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index c49c0f0e4bc2a..31d041b23e724 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -453,10 +453,9 @@ runSqlP2 (RunSQL t cascade _) = do return res isAltrOrDrop :: QErrM m => T.Text -> m Bool -isAltrOrDrop = either throwErr return . isMatchedE +isAltrOrDrop = either throwErr return . matchRegex regex False where throwErr s = throw500 $ "compiling regex failed: " <> T.pack s - isMatchedE = matchRegex regex False regex = "alter table|drop table|alter view|drop view|replace view" runRunSQL From e047146b68f2dbfcff7240227aa5ebc8adde89d9 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Fri, 18 Jan 2019 12:43:27 +0530 Subject: [PATCH 3/6] use 'ByteString' module for Regex, update enforce check field name --- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 22 +++++++++---------- server/src-lib/Hasura/Server/Utils.hs | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index 31d041b23e724..e871ef05e5245 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -376,9 +376,9 @@ buildSchemaCache = do data RunSQL = RunSQL - { rSql :: T.Text - , rCascade :: !(Maybe Bool) - , rEnforceStateCheck :: !(Maybe Bool) + { rSql :: T.Text + , rCascade :: !(Maybe Bool) + , rCheckMetadataConsistency :: !(Maybe Bool) } deriving (Show, Eq, Lift) $(deriveJSON (aesonDrop 1 snakeCase){omitNothingFields=True} ''RunSQL) @@ -452,22 +452,22 @@ runSqlP2 (RunSQL t cascade _) = do return res -isAltrOrDrop :: QErrM m => T.Text -> m Bool -isAltrOrDrop = either throwErr return . matchRegex regex False +isAltrDropReplace :: QErrM m => T.Text -> m Bool +isAltrDropReplace = either throwErr return . matchRegex regex False where throwErr s = throw500 $ "compiling regex failed: " <> T.pack s - regex = "alter table|drop table|alter view|drop view|replace view" + regex = "alter|drop|replace" runRunSQL :: (QErrM m, UserInfoM m, CacheRWM m, MonadTx m, MonadIO m, HasHttpManager m) => RunSQL -> m RespBody -runRunSQL q@(RunSQL t _ mEnfStChk) = do +runRunSQL q@(RunSQL t _ mChkMDCnstcy) = do adminOnly - encode <$> bool withoutStChk runP2 enfStChk + encode <$> bool withoutMDChk runP2 chkMDCnstcy where - enfStChk = fromMaybe False mEnfStChk - withoutStChk = - bool (execRawSQL t) runP2 =<< isAltrOrDrop t + chkMDCnstcy = fromMaybe False mChkMDCnstcy + withoutMDChk = + bool (execRawSQL t) runP2 =<< isAltrDropReplace t runP2 = runSqlP2 q -- Should be used only after checking the status diff --git a/server/src-lib/Hasura/Server/Utils.hs b/server/src-lib/Hasura/Server/Utils.hs index 25ead7a2bb7d2..a8d2e6067f87a 100644 --- a/server/src-lib/Hasura/Server/Utils.hs +++ b/server/src-lib/Hasura/Server/Utils.hs @@ -16,7 +16,7 @@ import qualified Data.Text.IO as TI import qualified Language.Haskell.TH.Syntax as TH import qualified Text.Ginger as TG import qualified Text.Regex.TDFA as TDFA -import qualified Text.Regex.TDFA.String as TDFA +import qualified Text.Regex.TDFA.ByteString as TDFA import Hasura.Prelude @@ -115,7 +115,7 @@ _3 :: (a, b, c) -> c _3 (_, _, z) = z -- regex related -matchRegex :: String -> Bool -> T.Text -> Either String Bool +matchRegex :: B.ByteString -> Bool -> T.Text -> Either String Bool matchRegex regex caseSensitive src = fmap (`TDFA.match` T.unpack src) compiledRegexE where From 62a69f74110f1d288ea6dd881a867ac3b8c96304 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Fri, 18 Jan 2019 13:07:31 +0530 Subject: [PATCH 4/6] only execute sql if 'check_metadata_consistency' is false --- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index e871ef05e5245..40721579bb67b 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -463,11 +463,13 @@ runRunSQL => RunSQL -> m RespBody runRunSQL q@(RunSQL t _ mChkMDCnstcy) = do adminOnly - encode <$> bool withoutMDChk runP2 chkMDCnstcy + encode <$> case mChkMDCnstcy of + Nothing -> withSQLChk + Just b -> bool execSQL runP2 b where - chkMDCnstcy = fromMaybe False mChkMDCnstcy - withoutMDChk = - bool (execRawSQL t) runP2 =<< isAltrDropReplace t + withSQLChk = + bool execSQL runP2 =<< isAltrDropReplace t + execSQL = execRawSQL t runP2 = runSqlP2 q -- Should be used only after checking the status From 96db6358ee7bea63b381c2f929c5cac73c968d2b Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Fri, 18 Jan 2019 14:17:00 +0530 Subject: [PATCH 5/6] simplify runRunSQL function --- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index 40721579bb67b..0f602a6bd1938 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -399,10 +399,10 @@ execRawSQL = let e = err400 PostgresError "query execution failed" in e {qeInternal = Just $ toJSON txe} -runSqlP2 +execWithMDCheck :: (QErrM m, CacheRWM m, MonadTx m, MonadIO m, HasHttpManager m) => RunSQL -> m RunSQLRes -runSqlP2 (RunSQL t cascade _) = do +execWithMDCheck (RunSQL t cascade _) = do -- Drop hdb_views so no interference is caused to the sql query liftTx $ Q.catchE defaultTxErrorHandler clearHdbViews @@ -463,14 +463,8 @@ runRunSQL => RunSQL -> m RespBody runRunSQL q@(RunSQL t _ mChkMDCnstcy) = do adminOnly - encode <$> case mChkMDCnstcy of - Nothing -> withSQLChk - Just b -> bool execSQL runP2 b - where - withSQLChk = - bool execSQL runP2 =<< isAltrDropReplace t - execSQL = execRawSQL t - runP2 = runSqlP2 q + isMDChkNeeded <- maybe (isAltrDropReplace t) return mChkMDCnstcy + encode <$> bool (execRawSQL t) (execWithMDCheck q) isMDChkNeeded -- Should be used only after checking the status resToCSV :: PQ.Result -> ExceptT T.Text IO [[T.Text]] From aed7b32a5d91b749c0a45e4bd943cd08a4e212d3 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Fri, 18 Jan 2019 14:32:42 +0530 Subject: [PATCH 6/6] regex match against 'ByteString' instead of 'String' --- server/src-lib/Hasura/Server/Utils.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src-lib/Hasura/Server/Utils.hs b/server/src-lib/Hasura/Server/Utils.hs index a8d2e6067f87a..2e3cf73ca7abc 100644 --- a/server/src-lib/Hasura/Server/Utils.hs +++ b/server/src-lib/Hasura/Server/Utils.hs @@ -117,7 +117,7 @@ _3 (_, _, z) = z -- regex related matchRegex :: B.ByteString -> Bool -> T.Text -> Either String Bool matchRegex regex caseSensitive src = - fmap (`TDFA.match` T.unpack src) compiledRegexE + fmap (`TDFA.match` TE.encodeUtf8 src) compiledRegexE where compOpt = TDFA.defaultCompOpt { TDFA.caseSensitive = caseSensitive