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 authored and steve-chavez committed May 21, 2024
1 parent e637f41 commit 529fa15
Show file tree
Hide file tree
Showing 22 changed files with 168 additions and 53 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ 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
Expand All @@ -23,6 +22,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
+ Shows the correct JSON format in the `hints` field
- #3340, Log when the LISTEN channel gets a notification - @steve-chavez
- #3184, Log full pg version to stderr on connection - @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 @@ -146,6 +147,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 @@ -241,6 +243,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 @@ -418,6 +421,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
1 change: 1 addition & 0 deletions src/PostgREST/Config/Database.hs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ dbSettingsNames =
,"db_root_spec"
,"db_schemas"
,"db_tx_end"
,"db_hoisted_tx_settings"
,"jwt_aud"
,"jwt_role_claim_key"
,"jwt_secret"
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, configDbHoistedTxSettings) (SchemaCache.accessibleFuncs pgVer configDbPreparedStatements)
<*> SQL.statement tSchema (SchemaCache.schemaDescription configDbPreparedStatements))
OAIgnorePriv ->
MaybeDbResult plan . Just <$> ((,,)
Expand Down
14 changes: 7 additions & 7 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, configDbHoistedTxSettings) $ allFunctions 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,13 +363,13 @@ 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 :: PgVersion -> Bool -> SQL.Statement ([Schema], [Text]) RoutineMap
allFunctions pgVer = SQL.Statement sql (contrazip2 (arrayParam HE.text) (arrayParam HE.text)) decodeFuncs
where
sql = funcsSqlQuery pgVer <> " AND pn.nspname = ANY($1)"

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

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),
substr(setting, strpos(setting, '=') + 1)
)) as kvs
FROM unnest(proconfig) setting
WHERE setting not LIKE 'default_transaction_isolation%'
WHERE setting ~ ANY($2)
) 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,32 @@
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 @@ -170,6 +171,26 @@
pdSchema: public
pdVolatility: Volatile

- - qiName: rpc_with_two_hoisted
qiSchema: public
- - pdDescription: null
pdFuncSettings:
- - 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 @@ -219,25 +240,6 @@
pdSchema: public
pdVolatility: Volatile

- - qiName: work_mem_test
qiSchema: public
- - pdDescription: null
pdFuncSettings:
- - work_mem
- 6000kB
pdHasVariadic: false
pdName: work_mem_test
pdParams: []
pdReturnType:
contents:
contents:
qiName: text
qiSchema: pg_catalog
tag: Scalar
tag: Single
pdSchema: public
pdVolatility: Volatile

- - qiName: invalid_role_claim_key_reload
qiSchema: public
- - pdDescription: null
Expand Down Expand Up @@ -348,7 +350,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 Expand Up @@ -454,6 +458,24 @@
pdSchema: public
pdVolatility: Volatile

- - qiName: rpc_work_mem
qiSchema: public
- - pdDescription: null
pdFuncSettings: []
pdHasVariadic: false
pdName: rpc_work_mem
pdParams: []
pdReturnType:
contents:
contents:
- qiName: items
qiSchema: public
- false
tag: Composite
tag: Single
pdSchema: public
pdVolatility: Volatile

- - qiName: four_sec_timeout
qiSchema: public
- - pdDescription: null
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 = "maintenance_work_mem"
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 = "work_mem"
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 = "work_mem"
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: work_mem
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 = "work_mem"
db-max-rows = 1000
db-plan-enabled = true
db-pool = 1
Expand Down
2 changes: 2 additions & 0 deletions test/io/db_config.sql
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ALTER ROLE db_config_authenticator SET pgrst.openapi_server_proxy_uri = 'https:/
ALTER ROLE db_config_authenticator SET pgrst.server_cors_allowed_origins = 'http://origin.com';
ALTER ROLE db_config_authenticator SET pgrst.server_timing_enabled = 'false';
ALTER ROLE db_config_authenticator SET pgrst.server_trace_header = 'CF-Ray';
ALTER ROLE db_config_authenticator SET pgrst.db_hoisted_tx_settings = 'work_mem';

-- override with database specific setting
ALTER ROLE db_config_authenticator IN DATABASE :DBNAME SET pgrst.db_extra_search_path = 'public, extensions, private';
Expand Down Expand Up @@ -72,6 +73,7 @@ ALTER ROLE other_authenticator SET pgrst.openapi_server_proxy_uri = 'https://oth
ALTER ROLE other_authenticator SET pgrst.server_cors_allowed_origins = 'http://otherorigin.com';
ALTER ROLE other_authenticator SET pgrst.server_timing_enabled = 'true';
ALTER ROLE other_authenticator SET pgrst.server_trace_header = 'traceparent';
ALTER ROLE other_authenticator SET pgrst.db_hoisted_tx_settings = 'maintenance_work_mem';

create schema postgrest;
grant usage on schema postgrest to db_config_authenticator;
Expand Down
Loading

0 comments on commit 529fa15

Please sign in to comment.