Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add config db-hoisted-tx-settings to allow only hoisted function settings #3358

Merged
merged 2 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
laurenceisla marked this conversation as resolved.
Show resolved Hide resolved
=============== ==================================================================================

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 = "autovacuum_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 = 'autovacuum_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