feat: apply all function settings as transaction-scoped settings

[taimoorzaeem]

Fixes #3061.

[taimoorzaeem]

queryFuncSettings :: PgVersion -> Bool -> Session FunctionSettings
queryFuncSettings 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
      func_setting AS (
        SELECT current_user AS rolname, unnest(p.proconfig) as setting
        FROM pg_proc p
      ),
      kv_settings AS (
        SELECT
          rolname,
          substr(setting, 1, strpos(setting, '=') - 1) AS key,
          lower(substr(setting, strpos(setting, '=') + 1)) AS value
        FROM func_setting
      ),
      SELECT
        kv.rolname,
        array_agg(row(kv.key, kv.value)) AS func_setting
        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|
      GROUP BY kv.rolname;

@steve-chavez With the function settings defined as:

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

The first ByteString here is role name. How are we going to obtain this? Unlike Impersonated Role Settings which have a rolename in pg_roles, we don't have a role name in pg_proc. Is the query alright?

[steve-chavez]

The first ByteString here is role name. How are we going to obtain this? Unlike Impersonated Role Settings which have a rolename in pg_roles, we don't have a role name in pg_proc. Is the query alright?

@taimoorzaeem We don't need a role here. Since the settings are defined in the function, we only need the settings from proconfig.

[taimoorzaeem]

@steve-chavez The type of FuncSettings is:

type FuncSettings = [(ByteString,ByteString,ByteString)]

I think a hashmap would be better performance-wise but I'm not sure.

[steve-chavez]

@taimoorzaeem How about having the function settings as part of:

postgrest/src/PostgREST/SchemaCache/Routine.hs

Lines 52 to 62 in d2fb67f

	data Routine = Function
	{ pdSchema :: Schema
	, pdName :: Text
	, pdDescription :: Maybe Text
	, pdParams :: [RoutineParam]
	, pdReturnType :: RetType
	, pdVolatility :: FuncVolatility
	, pdHasVariadic :: Bool
	, pdIsoLvl :: Maybe SQL.IsolationLevel
	}
	deriving (Eq, Show, Generic)

I think a hashmap would be better performance-wise but I'm not sure.

Then you wouldn't need to worry about search because the Function is already searched on Plan.callReadPlan, you would just need to get the funcSettings attribute.

---

Edit: Btw don't worry about the tests failures on older pg versions. We can figure that out later.

[steve-chavez]

@taimoorzaeem This won't work if the function has two settings like:

create or replace function four_sec_timeout() returns void as $$
  select pg_sleep(3);
$$ language sql
set statement_timeout = '4s'
set log_min_duration_sample = '5s';

[steve-chavez]

Just tested this and the query on this part should be like:

     coalesce(func_settings.kvs, '{}') as kvs

And below..

[steve-chavez]

Use this query here:

  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%'
  ) func_settings ON TRUE

This will output the settings like:

-[ RECORD 14 ]--------------+--------------------------------------------------------------
proc_schema                 | public
proc_name                   | four_sec_timeout
kvs                         | {"(statement_timeout,4s)","(log_min_duration_sample,5s)"}

[steve-chavez]

The decoding would then need to use compositeArrayColumn, like done here:

postgrest/src/PostgREST/Config/Database.hs

Lines 181 to 182 in 4958472

	rows :: HD.Result [(Text, Maybe Text, [(Text, Text)])]
	rows = HD.rowList $ (,,) <$> column HD.text <*> nullableColumn HD.text <*> compositeArrayColumn ((,) <$> compositeField HD.text <*> compositeField HD.text)

This won't need text splitting on the Haskell code.

[steve-chavez]

With the above changes then this type should be:

Suggested change

	type FuncSetting = (Text,Text)
	type FuncSettings = [(Text,Text)]

[steve-chavez]

Suggested change

	, pdFuncSetting :: Maybe FuncSetting
	, pdFuncSettings :: FuncSettings

[steve-chavez]

Since this causes an error on older pg versions, maybe you can use work_mem instead.

IIRC it won't require a GRANT.

Suggested change

	GRANT SET ON PARAMETER log_min_duration_sample TO postgrest_test_anonymous;
	create or replace function log_min_duration_test() returns text as $$
	select current_setting('log_min_duration_sample',false);
	$$ language sql set log_min_duration_sample = '5s';
	create or replace function work_mem_test() returns text as $$
	select current_setting('work_mem',false);
	$$ language sql set work_mem = '6000';

[steve-chavez]

Hm, the Build Linux static (Nix) CI job is failing with error: writing to file: No space left on device. I thought this would clear on main but didn't.

[wolfgangwalther]

I pushed one of the commits out of #3211 to main. Hopefully this fixes it for the moment. I am working on more things that should improve this, though.