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 9, 2024
1 parent 00ba76e commit 645c9af
Show file tree
Hide file tree
Showing 20 changed files with 157 additions and 38 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 @@ -181,7 +181,7 @@ actionQuery (MaybeDb plan@InspectPlan{ipSchema=tSchema}) AppConfig{..} _ pgVer s
tableAccess <- SQL.statement [tSchema] (SchemaCache.accessibleTables pgVer configDbPreparedStatements)
MaybeDbResult plan . Just <$> ((,,)
(HM.filterWithKey (\qi _ -> S.member qi tableAccess) $ SchemaCache.dbTables sCache)
<$> SQL.statement tSchema (SchemaCache.accessibleFuncs pgVer configDbPreparedStatements)
<$> SQL.statement tSchema (SchemaCache.accessibleFuncs configDbHoistedTxSettings pgVer configDbPreparedStatements)
<*> SQL.statement tSchema (SchemaCache.schemaDescription configDbPreparedStatements))
OAIgnorePriv ->
MaybeDbResult plan . Just <$> ((,,)
Expand Down
22 changes: 11 additions & 11 deletions src/PostgREST/SchemaCache.hs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ querySchemaCache AppConfig{..} = do
tabs <- SQL.statement schemas $ allTables pgVer prepared
keyDeps <- SQL.statement (schemas, configDbExtraSearchPath) $ allViewsKeyDependencies prepared
m2oRels <- SQL.statement mempty $ allM2OandO2ORels pgVer prepared
funcs <- SQL.statement schemas $ allFunctions pgVer prepared
funcs <- SQL.statement schemas $ allFunctions configDbHoistedTxSettings pgVer prepared
cRels <- SQL.statement mempty $ allComputedRels prepared
reps <- SQL.statement schemas $ dataRepresentations prepared
mHdlers <- SQL.statement schemas $ mediaHandlers pgVer prepared
Expand Down Expand Up @@ -363,18 +363,18 @@ dataRepresentations = SQL.Statement sql (arrayParam HE.text) decodeRepresentatio
OR (dst_t.typtype = 'd' AND c.castsource IN ('json'::regtype::oid , 'text'::regtype::oid)))
|]

allFunctions :: PgVersion -> Bool -> SQL.Statement [Schema] RoutineMap
allFunctions pgVer = SQL.Statement sql (arrayParam HE.text) decodeFuncs
allFunctions :: [Text] -> PgVersion -> Bool -> SQL.Statement [Schema] RoutineMap
allFunctions hoistedSets pgVer = SQL.Statement sql (arrayParam HE.text) decodeFuncs
where
sql = funcsSqlQuery pgVer <> " AND pn.nspname = ANY($1)"
sql = funcsSqlQuery hoistedSets pgVer <> " AND pn.nspname = ANY($1)"

accessibleFuncs :: PgVersion -> Bool -> SQL.Statement Schema RoutineMap
accessibleFuncs pgVer = SQL.Statement sql (param HE.text) decodeFuncs
accessibleFuncs :: [Text] -> PgVersion -> Bool -> SQL.Statement Schema RoutineMap
accessibleFuncs hoistedSets pgVer = SQL.Statement sql (param HE.text) decodeFuncs
where
sql = funcsSqlQuery pgVer <> " AND pn.nspname = $1 AND has_function_privilege(p.oid, 'execute')"
sql = funcsSqlQuery hoistedSets pgVer <> " AND pn.nspname = $1 AND has_function_privilege(p.oid, 'execute')"

funcsSqlQuery :: PgVersion -> SqlQuery
funcsSqlQuery pgVer = [q|
funcsSqlQuery :: [Text] -> PgVersion -> SqlQuery
funcsSqlQuery hoistedSets pgVer = [q|
-- Recursively get the base types of domains
WITH
base_types AS (
Expand Down Expand Up @@ -451,15 +451,15 @@ funcsSqlQuery pgVer = [q|
JOIN pg_namespace tn ON tn.oid = t.typnamespace
LEFT JOIN pg_class comp ON comp.oid = t.typrelid
LEFT JOIN pg_description as d ON d.objoid = p.oid
LEFT JOIN LATERAL unnest(proconfig) iso_config ON iso_config like 'default_transaction_isolation%'
LEFT JOIN LATERAL unnest(proconfig) iso_config ON iso_config LIKE 'default_transaction_isolation%'
LEFT JOIN LATERAL (
SELECT
array_agg(row(
substr(setting, 1, strpos(setting, '=') - 1),
lower(substr(setting, strpos(setting, '=') + 1))
)) as kvs
FROM unnest(proconfig) setting
WHERE setting not LIKE 'default_transaction_isolation%'
WHERE setting NOT LIKE ALL(ARRAY['|] <> encodeUtf8 (T.intercalate "," hoistedSets) <> [q|'])
) func_settings ON TRUE
WHERE t.oid <> 'trigger'::regtype AND COALESCE(a.callable, true)
|] <> (if pgVer >= pgVersion110 then "AND prokind = 'f'" else "AND NOT (proisagg OR proiswindow)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,31 +59,34 @@
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

- - qiName: serializable_isolation_level
qiSchema: public
- - pdDescription: null
pdFuncSettings: []
pdFuncSettings:
- - default_transaction_isolation
- serializable
pdHasVariadic: false
pdName: serializable_isolation_level
pdParams: []
Expand Down Expand Up @@ -165,6 +168,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 +251,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 Expand Up @@ -343,7 +369,9 @@
- - qiName: repeatable_read_isolation_level
qiSchema: public
- - pdDescription: null
pdFuncSettings: []
pdFuncSettings:
- - default_transaction_isolation
- repeatable read
pdHasVariadic: false
pdName: repeatable_read_isolation_level
pdParams: []
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;

0 comments on commit 645c9af

Please sign in to comment.