Re: Allow placeholders in ALTER ROLE w/o superuser
Pavel Borisov <pashkin.elfe@gmail.com>
From: Pavel Borisov <pashkin.elfe@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Steve Chavez <steve@supabase.io>, Tom Lane <tgl@sss.pgh.pa.us>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2022-12-05T11:18:07Z
Lists: pgsql-hackers
Attachments
- v4-0001-USER-SET-parameters-for-pg_db_role_setting.patch (application/octet-stream) patch v4-0001
> On Thu, Dec 1, 2022 at 6:14 AM Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Wed, Nov 23, 2022 at 1:53 AM Steve Chavez <steve@supabase.io> wrote: > > > So from my side this all looks good! > > > > Thank you for your feedback. > > > > The next revision of the patch is attached. It contains code > > improvements, comments and documentation. I'm going to also write > > sode tests. pg_db_role_setting doesn't seem to be well-covered with > > tests. I will probably need to write a new module into > > src/tests/modules to check now placeholders interacts with dynamically > > defined GUCs. > > Another revision of patch is attached. It's fixed now that USER SET > values can't be used for PGC_SUSET parameters. Tests are added. That > require new module test_pg_db_role_setting to check dynamically > defined GUCs. I've looked through the last version of a patch. The tests in v3 failed due to naming mismatches. I fixed this in v4 (PFA). The other thing that may seem unexpected: is whether the value should apply to the ordinary user only, encoded in the parameter name. The pro of this is that it doesn't break catalog compatibility by a separate field for GUC permissions a concept that doesn't exist today (and maybe not needed at all). Also, it makes the patch more minimalistic in the code. This is also fully compatible with the previous parameters naming due to parentheses being an unsupported symbol for the parameter name. I've also tried to revise the comments and docs a little bit to reflect the changes. The CI-enabled build of patch v4 for reference is at https://github.com/pashkinelfe/postgres/tree/placeholders-in-alter-role-v4 Overall the patch looks useful and good enough to be committed. Kind regards, Pavel Borisov, Supabase
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Remove extra regress check arguments from test_pg_db_role_setting
- 529da086ba7f 16.0 landed
-
meson: Add 'running' test setup, as a replacement for installcheck
- afdd9f7f0e00 16.0 landed
-
Add USER SET parameter values for pg_db_role_setting
- 096dd80f3ccc 16.0 landed
-
Fix sloppy cleanup of roles in privileges.sql.
- b62303794efd 16.0 cited
-
Drop test user when done with it.
- f31111bbe81d 12.0 cited
-
GRANT rights to CURRENT_USER instead of adding roles
- 6928484bda45 9.6.0 cited
-
Clean up roles from roleattributes test
- 072710dff3ee 9.6.0 cited
-
Make repeated 'make installcheck' runs work
- b22b77068380 9.5.0 cited