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

Apply all function settings as transaction-scoped settings #3061

Closed
steve-chavez opened this issue Nov 21, 2023 · 2 comments · Fixed by #3142
Closed

Apply all function settings as transaction-scoped settings #3061

steve-chavez opened this issue Nov 21, 2023 · 2 comments · Fixed by #3142
Assignees
Labels
difficulty: medium Haskell task involving PostgreSQL IO enhancement a feature, ready for implementation

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Nov 21, 2023

Problem

Currently, only statement_timeout is applied https://postgrest.org/en/latest/references/transactions.html#function-settings.

Applying other settings like plan_filter.statement_cost_limit is not possible.

CREATE OR REPLACE FUNCTION myfunc()
RETURNS void as $$
  SELECT pg_sleep(3); -- simulating some long-running process
$$
LANGUAGE SQL
SET plan_filter.statement_cost_limit TO 1300;

Solution

Apply all function settings.

Notes

Not necessary to test with plan_filter.statement_cost_limit, we can use some others like:

alter role postgrest_test_w_superuser_settings set log_min_duration_statement = 1;
alter role postgrest_test_w_superuser_settings set log_min_messages = 'fatal';

Also consider that applying some settings require privileges, see #3058

@steve-chavez steve-chavez added enhancement a feature, ready for implementation difficulty: medium Haskell task involving PostgreSQL IO labels Nov 21, 2023
@taimoorzaeem
Copy link
Collaborator

taimoorzaeem commented Dec 9, 2023

@steve-chavez Thoughts on implementing this? This would involve looking up and parsing all settings for each function which would mean that we have to keep a table for these settings and apply it. For statement_timeout, we did parsing in SchemeCache.hs like:

p.provariadic > 0 as hasvariadic,
lower((regexp_split_to_array((regexp_split_to_array(iso_config, '='))[2], ','))[1]) AS transaction_isolation_level,
lower((regexp_split_to_array((regexp_split_to_array(timeout_config, '='))[2], ','))[1]) AS statement_timeout
FROM pg_proc p

and also added this new setting to data Routine = .... That would be parsing 352 different pg_settings for all functions which seems like a lot. WDYT??

@steve-chavez
Copy link
Member Author

steve-chavez commented Dec 11, 2023

That would be parsing 352 different pg_settings for all functions which seems like a lot. WDYT??

@taimoorzaeem No, it should only be the function settings - i.e. the configs that are set when doing create function..., not all pg_settings. These can be obtained in a single query. Also we don't need to parse them in Haskell, they can be left as bytestring. The implementation should be similar to how it's done for roles. See:

type RoleSettings = (HM.HashMap ByteString (HM.HashMap ByteString ByteString))

queryRoleSettings :: PgVersion -> Bool -> Session (RoleSettings, RoleIsolationLvl)
queryRoleSettings pgVer prepared =
let transaction = if prepared then SQL.transaction else SQL.unpreparedTransaction in
transaction SQL.ReadCommitted SQL.Read $ SQL.statement mempty $ SQL.Statement sql HE.noParams (processRows <$> rows) prepared
where
sql = [q|
with
role_setting as (
select r.rolname, unnest(r.rolconfig) as setting
from pg_auth_members m
join pg_roles r on r.oid = m.roleid
where member = current_user::regrole::oid
),
kv_settings AS (
SELECT
rolname,
substr(setting, 1, strpos(setting, '=') - 1) as key,
lower(substr(setting, strpos(setting, '=') + 1)) as value
FROM role_setting
),
iso_setting AS (
SELECT rolname, value
FROM kv_settings
WHERE key = 'default_transaction_isolation'
)
select
kv.rolname,
i.value as iso_lvl,
coalesce(array_agg(row(kv.key, kv.value)) filter (where key <> 'default_transaction_isolation'), '{}') as role_settings
from kv_settings kv
join pg_settings ps on ps.name = kv.key |] <>
(if pgVer >= pgVersion150
then "and (ps.context = 'user' or has_parameter_privilege(current_user::regrole::oid, ps.name, 'set')) "
else "and ps.context = 'user' ") <> [q|
left join iso_setting i on i.rolname = kv.rolname
group by kv.rolname, i.value;
|]

Maybe:

 type FunctionSettings  = (HM.HashMap ByteString (HM.HashMap ByteString ByteString)) 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Haskell task involving PostgreSQL IO enhancement a feature, ready for implementation
Development

Successfully merging a pull request may close this issue.

2 participants