Skip to content

Commit

Permalink
feat: add config db-hoisted-tx-settings to allow only hoisted functio…
Browse files Browse the repository at this point in the history
…n settings
  • Loading branch information
taimoorzaeem committed Apr 7, 2024
1 parent 5ab317c commit 6ab54f4
Show file tree
Hide file tree
Showing 19 changed files with 146 additions and 25 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions docs/references/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/references/transactions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
5 changes: 5 additions & 0 deletions src/PostgREST/Config.hs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ data AppConfig = AppConfig
, configDbChannel :: Text
, configDbChannelEnabled :: Bool
, configDbExtraSearchPath :: [Text]
, configDbHoistedTxSettings :: [Text]
, configDbMaxRows :: Maybe Integer
, configDbPlanEnabled :: Bool
, configDbPoolSize :: Int
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/PostgREST/Query.hs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ setPgLocals dbActPlan 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,25 @@
pdSchema: public
pdVolatility: Volatile

- - qiName: multiple_func_settings_test
- - qiName: rpc_with_one_hoisted
qiSchema: public
- - pdDescription: null
pdFuncSettings:
- - work_mem
- '5000'
- - statement_timeout
- 10s
- 7s
pdHasVariadic: false
pdName: multiple_func_settings_test
pdName: rpc_with_one_hoisted
pdParams: []
pdReturnType:
contents:
contents:
qiName: record
qiSchema: pg_catalog
tag: Scalar
tag: SetOf
- qiName: items
qiSchema: public
- false
tag: Composite
tag: Single
pdSchema: public
pdVolatility: Volatile

Expand Down Expand Up @@ -165,6 +166,28 @@
pdSchema: public
pdVolatility: Volatile

- - qiName: rpc_with_two_hoisted
qiSchema: public
- - pdDescription: null
pdFuncSettings:
- - work_mem
- '5000'
- - statement_timeout
- 10s
pdHasVariadic: false
pdName: rpc_with_two_hoisted
pdParams: []
pdReturnType:
contents:
contents:
- qiName: items
qiSchema: public
- false
tag: Composite
tag: Single
pdSchema: public
pdVolatility: Volatile

- - qiName: set_statement_timeout
qiSchema: public
- - pdDescription: null
Expand Down Expand Up @@ -226,9 +249,10 @@
pdReturnType:
contents:
contents:
qiName: text
qiSchema: pg_catalog
tag: Scalar
- qiName: items
qiSchema: public
- false
tag: Composite
tag: Single
pdSchema: public
pdVolatility: Volatile
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/aliases.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/boolean-numeric.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/boolean-string.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/no-defaults-with-db.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/no-defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/expected/types.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/no-defaults-env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/io/configs/no-defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 20 additions & 6 deletions test/io/fixtures.sql
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,27 @@ create function get_postgres_version() returns int as $$
select current_setting('server_version_num')::int;
$$ language sql;

create or replace function work_mem_test() returns text as $$
select current_setting('work_mem',false);
$$ language sql set work_mem = '6000';
create or replace function work_mem_test() returns items as $$
select 1
$$ language sql
set work_mem = '6000';

create or replace function multiple_func_settings_test() returns setof record as $$
select current_setting('work_mem',false) as work_mem,
current_setting('statement_timeout',false) as statement_timeout;
create or replace function rpc_with_two_hoisted() returns items as $$
select 1
$$ language sql
set work_mem = '5000'
set statement_timeout = '10s';

create or replace function rpc_with_one_hoisted() returns items as $$
select 1
$$ language sql
set work_mem = '5000'
set statement_timeout = '7s';

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;
64 changes: 58 additions & 6 deletions test/io/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,12 @@ def test_role_settings(defaultenv):
)
assert response.text == '"10s"'

# reset statement timeout to original value
response = postgrest.session.post(
"/rpc/change_role_statement_timeout", data={"timeout": "2s"}
)
assert response.status_code == 204


def test_isolation_level(defaultenv):
"isolation_level should be set per role and per function"
Expand Down Expand Up @@ -1394,16 +1400,62 @@ 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:
response = postgrest.session.post("/rpc/work_mem_test")
env = {
**defaultenv,
"PGRST_DB_HOISTED_TX_SETTINGS": "work_mem",
}

assert response.text == '"6000kB"'
with run(env=env) as postgrest:
response = postgrest.session.get("/rpc/work_mem_test?select=get_work_mem")

assert response.text == '{"get_work_mem":"6000kB"}'


def test_multiple_func_settings(defaultenv):
"check multiple function settings are applied"

with run(env=defaultenv) as postgrest:
response = postgrest.session.post("/rpc/multiple_func_settings_test")
env = {
**defaultenv,
"PGRST_DB_HOISTED_TX_SETTINGS": "work_mem,statement_timeout",
}

with run(env=env) as postgrest:
response = postgrest.session.get(
"/rpc/rpc_with_two_hoisted?select=get_work_mem,get_statement_timeout"
)

assert (
response.text == '{"get_work_mem":"5000kB","get_statement_timeout":"10s"}'
)


def test_first_hoisted_setting_is_applied(defaultenv):
"test that work_mem is applied and statement_timeout is not 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?select=get_work_mem,get_statement_timeout"
)

assert response.text == '{"get_work_mem":"5000kB","get_statement_timeout":"2s"}'


def test_second_hoisted_setting_is_applied(defaultenv):
"test that statement_timeout is applied and work_mem is not applied"

env = {
**defaultenv,
"PGRST_DB_HOISTED_TX_SETTINGS": "statement_timeout",
}

with run(env=env) as postgrest:
response = postgrest.session.get(
"/rpc/rpc_with_one_hoisted?select=get_work_mem,get_statement_timeout"
)

assert response.text == '[{"work_mem":"5000kB","statement_timeout":"10s"}]'
assert response.text == '{"get_work_mem":"4MB","get_statement_timeout":"7s"}'
1 change: 1 addition & 0 deletions test/spec/SpecHelper.hs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ baseCfg = let secret = Just $ encodeUtf8 "reallyreallyreallyreallyverysafe" in
, configDbChannel = mempty
, configDbChannelEnabled = True
, configDbExtraSearchPath = []
, configDbHoistedTxSettings = []
, configDbMaxRows = Nothing
, configDbPlanEnabled = False
, configDbPoolSize = 10
Expand Down

0 comments on commit 6ab54f4

Please sign in to comment.