Transaction-Scoped Settings are unclear on the logs

[steve-chavez]

Problem

Transaction-Scoped Settings show on the logs as:

select set_config($1, $2, true), set_config($3, $4, true), set_config($5, $6, true), set_config($7, $8, true), set_config($9, $10, true), set_config($11, $12, true), set_config($13, $14, true);

Details do show the parameters:

2023-10-21 13:40:10.955 -05 [498923] DETAIL:  parameters: $1 = 'search_path', $2 = '"test", "public"', $3 = 'role', $4 = 'postgrest_test_anonymous', $5 = 'request.jwt.claims', $6 = '{"role":"p
ostgrest_test_anonymous"}', $7 = 'request.method', $8 = 'GET', $9 = 'request.path', $10 = '/projects', $11 = 'request.headers', $12 = '{"host":"localhost:3000","accept":"*/*","user-agent":"cur
l/7.81.0"}', $13 = 'request.cookies', $14 = '{}'

But this level of verbosity is not always enabled. Also they're not clear in pg_stat_statements.

Solution

The first parameter of the set_config doesn't need to be parametrized, as it's always defined by us.

So we can convert them to:

select set_config('search_path', $1, true), set_config('role', $2, true), set_config('request.jwt.claims', $3, true), set_config('request.method', $4, true) ...

[laurenceisla]

I've been working in this and noticed these cases:

• There are values that can be set by the user in a request when db-use-legacy-gucs is enabled for Postgres 13 and below.
• Also, these settings are done by the user in the DB and the config file/env vars:
  

  postgrest/src/PostgREST/Query.hs

  Lines 259 to 260 in 5c9c7f4

  	roleSettingsSql = setConfigLocal mempty <$> roleSettings
  	appSettingsSql = setConfigLocal mempty <$> (join bimap toUtf8 <$> configAppSettings)

E.g. app-settings-anysetting = ...

Aside from those the rest are hardcoded directly. Not sure about the second case but the first is a no-go. Would it be OK to leave those parameterized while the others are not? The problem of unclear logs may persist/aggravate that way.

Or maybe do not apply this if there's legacy GUCs? Still not convinced about the second case, though.

[steve-chavez]

There are values that can be set by the user in a request when db-use-legacy-gucs is enabled for Postgres 13 and below.

@laurenceisla Re db-use-legacy-gucs, this might be a good opportunity (since we're going to do a major version) to drop the config altogether and only use the new JSON gucs.

Also, these settings are done by the user in the DB and the config file/env vars:

These could use some light escaping on our side. But how about leaving them for a second iteration? I think first we can get rid of legacy gucs and clarify the hardcoded settings.

[gmichalec-pandora]

In case anyone else stumbles on this - this change breaks support for the old 'request.jwt.claim.jti'. The new syntax can be seen here: https://postgrest.org/en/v12/references/auth.html#custom-validation
I wish there had been a clearer note in the changelog - it took me quite a while to figure out why weren't logging user transactions. Removed db-use-legacy-gucs doesn't tell you much if you don't have the context . I appreciate the work done here, but clearly documenting breaking changes helps us be more confident in performing upgrades.

[steve-chavez]

@gmichalec-pandora You're right, thank you. We made the clarification on #3287