Skip to content

Commit

Permalink
perf: Pass arguments to RPCs called via GET directly
Browse files Browse the repository at this point in the history
Previously they were passed as a JSON payload. This results in a LATERAL
join for the calling expression, which prevents LIMIT from being pushed
into the inlined function call, making some requests really slow.

Resolves #2858
  • Loading branch information
wolfgangwalther committed Jul 9, 2024
1 parent 0dc1345 commit ee4bfbf
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

- #3558, Add the `admin-server-host` config to set the host for the admin server - @develop7
- #3607, Log to stderr when the JWT secret is less than 32 characters long - @laurenceisla
- #2858, Performance improvements when calling RPCs via GET using indexes in more cases - @wolfgangwalther

### Fixed

Expand Down
11 changes: 5 additions & 6 deletions src/PostgREST/Plan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ module PostgREST.Plan
, CallReadPlan(..)
) where

import qualified Data.ByteString.Lazy as LBS
import qualified Data.HashMap.Strict as HM
import qualified Data.HashMap.Strict.InsOrd as HMI
import qualified Data.List as L
Expand Down Expand Up @@ -176,9 +175,9 @@ callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferenc
let relIdentifier = QualifiedIdentifier pdSchema (fromMaybe pdName $ Routine.funcTableName proc) -- done so a set returning function can embed other relations
rPlan <- readPlan relIdentifier conf sCache apiRequest
let args = case (invMethod, iContentMediaType) of
(InvRead _, _) -> jsonRpcParams proc qsParams'
(Inv, MTUrlEncoded) -> maybe mempty (jsonRpcParams proc . payArray) iPayload
(Inv, _) -> maybe mempty payRaw iPayload
(InvRead _, _) -> DirectArgs $ toRpcParams proc qsParams'
(Inv, MTUrlEncoded) -> DirectArgs $ maybe mempty (toRpcParams proc . payArray) iPayload
(Inv, _) -> JsonArgs $ payRaw <$> iPayload
txMode = case (invMethod, pdVolatility) of
(InvRead _, _) -> SQL.Read
(Inv, Routine.Stable) -> SQL.Read
Expand Down Expand Up @@ -955,11 +954,11 @@ resolveOrError ctx (Just table) field =
CoercibleField{cfIRType=""} -> Left $ ColumnNotFound (tableName table) field
cf -> Right $ withJsonParse ctx cf

callPlan :: Routine -> ApiRequest -> S.Set FieldName -> LBS.ByteString -> ReadPlanTree -> CallPlan
callPlan :: Routine -> ApiRequest -> S.Set FieldName -> CallArgs -> ReadPlanTree -> CallPlan
callPlan proc ApiRequest{iPreferences=Preferences{..}} paramKeys args readReq = FunctionCall {
funCQi = QualifiedIdentifier (pdSchema proc) (pdName proc)
, funCParams = callParams
, funCArgs = Just args
, funCArgs = args
, funCScalar = funcReturnsScalar proc
, funCSetOfScalar = funcReturnsSetOfScalar proc
, funCRetCompositeAlias = funcReturnsCompositeAlias proc
Expand Down
37 changes: 22 additions & 15 deletions src/PostgREST/Plan/CallPlan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
module PostgREST.Plan.CallPlan
( CallPlan(..)
, CallParams(..)
, jsonRpcParams
, CallArgs(..)
, RpcParamValue(..)
, toRpcParams
)
where

Expand All @@ -19,7 +21,7 @@ import Protolude
data CallPlan = FunctionCall
{ funCQi :: QualifiedIdentifier
, funCParams :: CallParams
, funCArgs :: Maybe LBS.ByteString
, funCArgs :: CallArgs
, funCScalar :: Bool
, funCSetOfScalar :: Bool
, funCRetCompositeAlias :: Bool
Expand All @@ -30,14 +32,26 @@ data CallParams
= KeyParams [RoutineParam] -- ^ Call with key params: func(a := val1, b:= val2)
| OnePosParam RoutineParam -- ^ Call with positional params(only one supported): func(val)

data CallArgs
= DirectArgs (HM.HashMap Text RpcParamValue)
| JsonArgs (Maybe LBS.ByteString)

-- | RPC query param value `/rpc/func?v=<value>`, used for VARIADIC functions on form-urlencoded POST and GETs
-- | It can be fixed `?v=1` or repeated `?v=1&v=2&v=3.
data RpcParamValue = Fixed Text | Variadic [Text]
instance JSON.ToJSON RpcParamValue where
toJSON (Fixed v) = JSON.toJSON v
-- Not possible to get here anymore. Variadic arguments are only supported for
-- true variadic arguments, but the toJSON instance is only used for the "single unnamed json argument" case.
toJSON (Variadic v) = JSON.toJSON v

-- | Convert rpc params `/rpc/func?a=val1&b=val2` to json `{"a": "val1", "b": "val2"}
jsonRpcParams :: Routine -> [(Text, Text)] -> LBS.ByteString
jsonRpcParams proc prms =
if not $ pdHasVariadic proc then -- if proc has no variadic param, save steps and directly convert to json
JSON.encode $ HM.fromList $ second JSON.toJSON <$> prms
toRpcParams :: Routine -> [(Text, Text)] -> HM.HashMap Text RpcParamValue
toRpcParams proc prms =
if not $ pdHasVariadic proc then -- if proc has no variadic param, save steps and directly convert to map
HM.fromList $ second Fixed <$> prms
else
let paramsMap = HM.fromListWith mergeParams $ toRpcParamValue proc <$> prms in
JSON.encode paramsMap
HM.fromListWith mergeParams $ toRpcParamValue proc <$> prms
where
mergeParams :: RpcParamValue -> RpcParamValue -> RpcParamValue
mergeParams (Variadic a) (Variadic b) = Variadic $ b ++ a
Expand All @@ -48,10 +62,3 @@ toRpcParamValue proc (k, v) | prmIsVariadic k = (k, Variadic [v])
| otherwise = (k, Fixed v)
where
prmIsVariadic prm = isJust $ find (\RoutineParam{ppName, ppVar} -> ppName == prm && ppVar) $ pdParams proc

-- | RPC query param value `/rpc/func?v=<value>`, used for VARIADIC functions on form-urlencoded POST and GETs
-- | It can be fixed `?v=1` or repeated `?v=1&v=2&v=3.
data RpcParamValue = Fixed Text | Variadic [Text]
instance JSON.ToJSON RpcParamValue where
toJSON (Fixed v) = JSON.toJSON v
toJSON (Variadic v) = JSON.toJSON v
32 changes: 29 additions & 3 deletions src/PostgREST/Query/QueryBuilder.hs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{-# LANGUAGE DuplicateRecordFields #-}
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE RecordWildCards #-}
{-|
Module : PostgREST.Query.QueryBuilder
Description : PostgREST SQL queries generating functions.
Expand All @@ -16,8 +17,11 @@ module PostgREST.Query.QueryBuilder
, limitedQuery
) where

import qualified Data.Aeson as JSON
import qualified Data.ByteString.Char8 as BS
import qualified Data.HashMap.Strict as HM
import qualified Hasql.DynamicStatements.Snippet as SQL
import qualified Hasql.Encoders as HE

import Data.Maybe (fromJust)
import Data.Tree (Tree (..))
Expand Down Expand Up @@ -190,14 +194,19 @@ mutatePlanToQuery (Delete mainQi logicForest range ordts returnings)
(whereRangeIdF, rangeIdF) = mutRangeF mainQi (cfName . coField <$> ordts)

callPlanToQuery :: CallPlan -> PgVersion -> SQL.Snippet
callPlanToQuery (FunctionCall qi params args returnsScalar returnsSetOfScalar returnsCompositeAlias returnings) pgVer =
callPlanToQuery (FunctionCall qi params arguments returnsScalar returnsSetOfScalar returnsCompositeAlias returnings) pgVer =
"SELECT " <> (if returnsScalar || returnsSetOfScalar then "pgrst_call.pgrst_scalar" else returnedColumns) <> " " <>
fromCall
where
jsonArgs = case arguments of
DirectArgs args -> Just $ JSON.encode args
JsonArgs json -> json
fromCall = case params of
OnePosParam prm -> "FROM " <> callIt (singleParameter args $ encodeUtf8 $ ppType prm)
OnePosParam prm -> "FROM " <> callIt (singleParameter jsonArgs $ encodeUtf8 $ ppType prm)
KeyParams [] -> "FROM " <> callIt mempty
KeyParams prms -> fromJsonBodyF args ((\p -> CoercibleField (ppName p) mempty False (ppTypeMaxLength p) Nothing Nothing) <$> prms) False True False <> ", " <>
KeyParams prms -> case arguments of
DirectArgs args -> "FROM " <> callIt (fmtArgs prms args)
JsonArgs json -> fromJsonBodyF json ((\p -> CoercibleField (ppName p) mempty False (ppTypeMaxLength p) Nothing Nothing) <$> prms) False True False <> ", " <>
"LATERAL " <> callIt (fmtParams prms)

callIt :: SQL.Snippet -> SQL.Snippet
Expand All @@ -209,6 +218,23 @@ callPlanToQuery (FunctionCall qi params args returnsScalar returnsSetOfScalar re
fmtParams prms = intercalateSnippet ", "
((\a -> (if ppVar a then "VARIADIC " else mempty) <> pgFmtIdent (ppName a) <> " := pgrst_body." <> pgFmtIdent (ppName a)) <$> prms)

fmtArgs :: [RoutineParam] -> HM.HashMap Text RpcParamValue -> SQL.Snippet
fmtArgs prms args = intercalateSnippet ", " $ fmtArg <$> prms
where
fmtArg RoutineParam{..} =
(if ppVar then "VARIADIC " else mempty) <>
pgFmtIdent ppName <>
" := " <>
encodeArg (HM.lookup ppName args) <>
"::" <>
SQL.sql (encodeUtf8 ppTypeMaxLength)
encodeArg :: Maybe RpcParamValue -> SQL.Snippet
encodeArg (Just (Variadic v)) = SQL.encoderAndParam (HE.nonNullable $ HE.foldableArray $ HE.nonNullable HE.text) v
encodeArg (Just (Fixed v)) = SQL.encoderAndParam (HE.nonNullable HE.unknown) $ encodeUtf8 v
-- Currently not supported: Calling functions without some of their arguments without DEFAULT.
-- We could fallback to providing this NULL value in those cases.
encodeArg Nothing = "NULL"

returnedColumns :: SQL.Snippet
returnedColumns
| null returnings = "*"
Expand Down
4 changes: 2 additions & 2 deletions test/spec/Feature/Query/PlanSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ spec actualPgVersion = do
r <- request methodGet "/rpc/get_projects_below?id=3"
[planHdr] ""

liftIO $ planCost r `shouldSatisfy` (< 45.4)
liftIO $ planCost r `shouldSatisfy` (< 35.4)

it "should not exceed cost when calling setof composite proc with empty params" $ do
r <- request methodGet "/rpc/getallprojects"
Expand All @@ -360,7 +360,7 @@ spec actualPgVersion = do
r <- request methodGet "/rpc/add_them?a=3&b=4"
[planHdr] ""

liftIO $ planCost r `shouldSatisfy` (< 0.11)
liftIO $ planCost r `shouldSatisfy` (< 0.08)

context "function inlining" $ do
it "should inline a zero argument function(the function won't appear in the plan tree)" $ do
Expand Down

0 comments on commit ee4bfbf

Please sign in to comment.