diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cddd98c048..6004c0d1a12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2887, Add Preference `max-affected` to limit affected resources - @taimoorzaeem - #3171, Add an ability to dump config via admin API - @skywriter - - #3061, Apply all function settings as transaction-scoped settings - @taimoorzaeem - #3171, #3046, Log schema cache stats to stderr - @steve-chavez - #3210, Dump schema cache through admin API - @taimoorzaeem - #2676, Performance improvement on bulk json inserts, around 10% increase on requests per second by removing `json_typeof` from write queries - @steve-chavez + - #3242. Add config `db-hoisted-tx-settings` to apply only hoisted function settings - @taimoorzaeem ### Fixed diff --git a/docs/references/configuration.rst b/docs/references/configuration.rst index 2067f38be5c..a3acebd11aa 100644 --- a/docs/references/configuration.rst +++ b/docs/references/configuration.rst @@ -298,6 +298,21 @@ db-extra-search-path Multiple schemas can be added in a comma-separated string, e.g. ``public, extensions``. +.. _db-hoisted-tx-settings: + +db-hoisted-tx-settings +---------------------- + + =============== ================================================================================== + **Type** String + **Default** statement_timeout, plan_filter.statement_cost_limit, default_transaction_isolation + **Reloadable** Y + **Environment** PGRST_DB_HOISTED_TX_SETTINGS + **In-Database** pgrst.db_hoisted_tx_settings + =============== ================================================================================== + + Hoisted settings are allowed to be applied as transaction-scoped function settings. Multiple settings can be added in a comma-separated string, e.g. ``work_mem, statement_timeout``. + .. _db-max-rows: db-max-rows diff --git a/docs/references/transactions.rst b/docs/references/transactions.rst index a6922eb0a74..6bef2b74b69 100644 --- a/docs/references/transactions.rst +++ b/docs/references/transactions.rst @@ -303,7 +303,7 @@ When calling the above function (see :ref:`functions`), the statement timeout wi .. note:: - Currently, only ``statement_timeout`` is applied for functions. + Only the transactions that are hoisted by config :ref:`db-hoisted-tx-settings` will be applied. .. _main_query: diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index 6d4de526da7..aa05b310df4 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -74,6 +74,7 @@ data AppConfig = AppConfig , configDbChannel :: Text , configDbChannelEnabled :: Bool , configDbExtraSearchPath :: [Text] + , configDbHoistedTxSettings :: [Text] , configDbMaxRows :: Maybe Integer , configDbPlanEnabled :: Bool , configDbPoolSize :: Int @@ -145,6 +146,7 @@ toText conf = ,("db-channel", q . configDbChannel) ,("db-channel-enabled", T.toLower . show . configDbChannelEnabled) ,("db-extra-search-path", q . T.intercalate "," . configDbExtraSearchPath) + ,("db-hoisted-tx-settings", q . T.intercalate "," . configDbHoistedTxSettings) ,("db-max-rows", maybe "\"\"" show . configDbMaxRows) ,("db-plan-enabled", T.toLower . show . configDbPlanEnabled) ,("db-pool", show . configDbPoolSize) @@ -240,6 +242,7 @@ parser optPath env dbSettings roleSettings roleIsolationLvl = <*> (fromMaybe "pgrst" <$> optString "db-channel") <*> (fromMaybe True <$> optBool "db-channel-enabled") <*> (maybe ["public"] splitOnCommas <$> optValue "db-extra-search-path") + <*> (maybe defaultHoistedAllowList splitOnCommas <$> optValue "db-hoisted-tx-settings") <*> optWithAlias (optInt "db-max-rows") (optInt "max-rows") <*> (fromMaybe False <$> optBool "db-plan-enabled") @@ -416,6 +419,8 @@ parser optPath env dbSettings roleSettings roleIsolationLvl = splitOnCommas (C.String s) = T.strip <$> T.splitOn "," s splitOnCommas _ = [] + defaultHoistedAllowList = ["statement_timeout","plan_filter.statement_cost_limit","default_transaction_isolation"] + -- | Read the JWT secret from a file if configJwtSecret is actually a -- filepath(has @ as its prefix). To check if the JWT secret is provided is -- in fact a file path, it must be decoded as 'Text' to be processed. diff --git a/src/PostgREST/Query.hs b/src/PostgREST/Query.hs index dee6f9c270d..a4f555b293a 100644 --- a/src/PostgREST/Query.hs +++ b/src/PostgREST/Query.hs @@ -256,7 +256,7 @@ setPgLocals actPlan AppConfig{..} claims role ApiRequest{..} = lift $ roleSettingsSql = setConfigWithDynamicName <$> HM.toList (fromMaybe mempty $ HM.lookup role configRoleSettings) appSettingsSql = setConfigWithDynamicName <$> (join bimap toUtf8 <$> configAppSettings) timezoneSql = maybe mempty (\(PreferTimezone tz) -> [setConfigWithConstantName ("timezone", tz)]) $ preferTimezone iPreferences - funcSettingsSql = setConfigWithDynamicName <$> (join bimap toUtf8 <$> funcSettings) + funcSettingsSql = setConfigWithDynamicName <$> (join bimap toUtf8 <$> filter (\(a,_) -> a `elem` configDbHoistedTxSettings) funcSettings) searchPathSql = let schemas = escapeIdentList (iSchema : configDbExtraSearchPath) in setConfigWithConstantName ("search_path", schemas) diff --git a/test/io/configs/expected/aliases.config b/test/io/configs/expected/aliases.config index bf2df05a115..00d9414e05b 100644 --- a/test/io/configs/expected/aliases.config +++ b/test/io/configs/expected/aliases.config @@ -3,6 +3,7 @@ db-anon-role = "" db-channel = "pgrst" db-channel-enabled = true db-extra-search-path = "public" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit,default_transaction_isolation" db-max-rows = 1000 db-plan-enabled = false db-pool = 10 diff --git a/test/io/configs/expected/boolean-numeric.config b/test/io/configs/expected/boolean-numeric.config index 1359f09fef3..5c025e0c167 100644 --- a/test/io/configs/expected/boolean-numeric.config +++ b/test/io/configs/expected/boolean-numeric.config @@ -3,6 +3,7 @@ db-anon-role = "" db-channel = "pgrst" db-channel-enabled = true db-extra-search-path = "public" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit,default_transaction_isolation" db-max-rows = "" db-plan-enabled = false db-pool = 10 diff --git a/test/io/configs/expected/boolean-string.config b/test/io/configs/expected/boolean-string.config index 1359f09fef3..5c025e0c167 100644 --- a/test/io/configs/expected/boolean-string.config +++ b/test/io/configs/expected/boolean-string.config @@ -3,6 +3,7 @@ db-anon-role = "" db-channel = "pgrst" db-channel-enabled = true db-extra-search-path = "public" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit,default_transaction_isolation" db-max-rows = "" db-plan-enabled = false db-pool = 10 diff --git a/test/io/configs/expected/defaults.config b/test/io/configs/expected/defaults.config index aefd98aa0d4..1d0c095d618 100644 --- a/test/io/configs/expected/defaults.config +++ b/test/io/configs/expected/defaults.config @@ -3,6 +3,7 @@ db-anon-role = "" db-channel = "pgrst" db-channel-enabled = true db-extra-search-path = "public" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit,default_transaction_isolation" db-max-rows = "" db-plan-enabled = false db-pool = 10 diff --git a/test/io/configs/expected/no-defaults-with-db-other-authenticator.config b/test/io/configs/expected/no-defaults-with-db-other-authenticator.config index ffa9fd5280c..d203891143c 100644 --- a/test/io/configs/expected/no-defaults-with-db-other-authenticator.config +++ b/test/io/configs/expected/no-defaults-with-db-other-authenticator.config @@ -3,6 +3,7 @@ db-anon-role = "pre_config_role" db-channel = "postgrest" db-channel-enabled = false db-extra-search-path = "public,extensions,other" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit" db-max-rows = 100 db-plan-enabled = true db-pool = 1 diff --git a/test/io/configs/expected/no-defaults-with-db.config b/test/io/configs/expected/no-defaults-with-db.config index a78347593c6..0f31856ac82 100644 --- a/test/io/configs/expected/no-defaults-with-db.config +++ b/test/io/configs/expected/no-defaults-with-db.config @@ -3,6 +3,7 @@ db-anon-role = "anonymous" db-channel = "postgrest" db-channel-enabled = false db-extra-search-path = "public,extensions,private" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit" db-max-rows = 500 db-plan-enabled = false db-pool = 1 diff --git a/test/io/configs/expected/no-defaults.config b/test/io/configs/expected/no-defaults.config index 2a45b21df00..b6ebf4426c1 100644 --- a/test/io/configs/expected/no-defaults.config +++ b/test/io/configs/expected/no-defaults.config @@ -3,6 +3,7 @@ db-anon-role = "root" db-channel = "postgrest" db-channel-enabled = false db-extra-search-path = "public,test" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit" db-max-rows = 1000 db-plan-enabled = true db-pool = 1 diff --git a/test/io/configs/expected/types.config b/test/io/configs/expected/types.config index e6d0328b657..b363c94bb41 100644 --- a/test/io/configs/expected/types.config +++ b/test/io/configs/expected/types.config @@ -3,6 +3,7 @@ db-anon-role = "" db-channel = "pgrst" db-channel-enabled = true db-extra-search-path = "public" +db-hoisted-tx-settings = "statement_timeout,plan_filter.statement_cost_limit,default_transaction_isolation" db-max-rows = "" db-plan-enabled = false db-pool = 10 diff --git a/test/io/configs/no-defaults-env.yaml b/test/io/configs/no-defaults-env.yaml index 4f03111a28d..5b57f71714c 100644 --- a/test/io/configs/no-defaults-env.yaml +++ b/test/io/configs/no-defaults-env.yaml @@ -5,6 +5,7 @@ PGRST_DB_ANON_ROLE: root PGRST_DB_CHANNEL: postgrest PGRST_DB_CHANNEL_ENABLED: false PGRST_DB_EXTRA_SEARCH_PATH: public, test +PGRST_DB_HOISTED_TX_SETTINGS: statement_timeout, plan_filter.statement_cost_limit PGRST_DB_MAX_ROWS: 1000 PGRST_DB_PLAN_ENABLED: true PGRST_DB_POOL: 1 diff --git a/test/io/configs/no-defaults.config b/test/io/configs/no-defaults.config index d9944c352a3..462e51cdfc7 100644 --- a/test/io/configs/no-defaults.config +++ b/test/io/configs/no-defaults.config @@ -3,6 +3,7 @@ db-anon-role = "root" db-channel = "postgrest" db-channel-enabled = false db-extra-search-path = "public, test" +db-hoisted-tx-settings = "statement_timeout, plan_filter.statement_cost_limit" db-max-rows = 1000 db-plan-enabled = true db-pool = 1 diff --git a/test/io/fixtures.sql b/test/io/fixtures.sql index 636505664d5..be449990177 100644 --- a/test/io/fixtures.sql +++ b/test/io/fixtures.sql @@ -213,3 +213,18 @@ create or replace function multiple_func_settings_test() returns setof record as $$ language sql set work_mem = '5000' set statement_timeout = '10s'; + +-- used with computed fields +create function rpc_with_one_hoisted_setting() returns items as $$ + select 1 +$$ language sql +set work_mem = '5000' +set statement_timeout = '5s'; + +create function get_work_mem(items) returns text as $$ + select current_setting('work_mem', true) as work_mem +$$ language sql; + +create function get_statement_timeout(items) returns text as $$ + select current_setting('statement_timeout', true) as statement_timeout +$$ language sql; diff --git a/test/io/test_io.py b/test/io/test_io.py index bf48ddb1605..2ad4221bd8b 100644 --- a/test/io/test_io.py +++ b/test/io/test_io.py @@ -1394,7 +1394,12 @@ def test_function_setting_statement_timeout_passes(defaultenv): def test_function_setting_work_mem(defaultenv): "check function setting work_mem is applied" - with run(env=defaultenv) as postgrest: + env = { + **defaultenv, + "PGRST_DB_HOISTED_TX_SETTINGS": "work_mem", + } + + with run(env=env) as postgrest: response = postgrest.session.post("/rpc/work_mem_test") assert response.text == '"6000kB"' @@ -1403,7 +1408,28 @@ def test_function_setting_work_mem(defaultenv): def test_multiple_func_settings(defaultenv): "check multiple function settings are applied" - with run(env=defaultenv) as postgrest: + env = { + **defaultenv, + "PGRST_DB_HOISTED_TX_SETTINGS": "work_mem,statement_timeout", + } + + with run(env=env) as postgrest: response = postgrest.session.post("/rpc/multiple_func_settings_test") assert response.text == '[{"work_mem":"5000kB","statement_timeout":"10s"}]' + + +def test_only_hoisted_settings_are_applied(defaultenv): + "test that only hoisted function settings are applied" + + env = { + **defaultenv, + "PGRST_DB_HOISTED_TX_SETTINGS": "work_mem", # only work_mem is hoisted + } + + with run(env=env) as postgrest: + response = postgrest.session.get( + "/rpc/rpc_with_one_hoisted_setting?select=get_work_mem,get_statement_timeout" + ) + + assert response.text == '{"get_work_mem":"5000kB","get_statement_timeout":"2s"}' diff --git a/test/spec/SpecHelper.hs b/test/spec/SpecHelper.hs index 0717ef12b4d..5175822f90c 100644 --- a/test/spec/SpecHelper.hs +++ b/test/spec/SpecHelper.hs @@ -103,6 +103,7 @@ baseCfg = let secret = Just $ encodeUtf8 "reallyreallyreallyreallyverysafe" in , configDbChannel = mempty , configDbChannelEnabled = True , configDbExtraSearchPath = [] + , configDbHoistedTxSettings = [] , configDbMaxRows = Nothing , configDbPlanEnabled = False , configDbPoolSize = 10