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

Allow define a search_path on the database #401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NitriKx
Copy link

@NitriKx NitriKx commented Feb 2, 2024

In this PR I've added a search_path parameter on the postgresql_database resource, which allows to alter the database to set a value for its search path.

It's inspired from what has been done by on the role (the readSearchPath function has been moved in helpers.go as I've re-used it.

Web documentation has been updated as well 🙂

@NitriKx NitriKx marked this pull request as ready for review February 2, 2024 09:11
@NitriKx NitriKx changed the title allow define a search_path on the database Allow define a search_path on the database Feb 2, 2024

var searchPathStr string
err = db.QueryRow(`
SELECT (pg_options_to_table(drs.setconfig)).option_value
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL pg_options_to_table 👀


func getDBRoleSettings(db *DBConnection, dbId string) (pq.ByteaArray, error) {
var dbRoleConfigItems pq.ByteaArray
dbSQL := `SELECT setconfig FROM pg_catalog.pg_database AS d, pg_catalog.pg_db_role_setting AS drs WHERE d.datname = $1 AND d.oid = drs.setdatabase`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dbSQL := `SELECT setconfig FROM pg_catalog.pg_database AS d, pg_catalog.pg_db_role_setting AS drs WHERE d.datname = $1 AND d.oid = drs.setdatabase`
dbSQL := `SELECT setconfig FROM pg_catalog.pg_database AS d, pg_catalog.pg_db_role_setting AS drs WHERE d.datname = $1 AND d.oid = drs.setdatabase AND setrole = 0`

I think you need to filter on setrole = 0 too, otherwise if someone do ALTER ROLE x IN DATABASE y SET search_path = 'test' on this database, this query will have multiple results.

e.g.:

postgres=# create database test; create role test;
CREATE DATABASE
CREATE ROLE

postgres=# alter database test set search_path = 'test'; alter role test in database test set search_path = 'other';
ALTER DATABASE
ALTER ROLE

postgres=# SELECT drs.* FROM pg_catalog.pg_database AS d, pg_catalog.pg_db_role_setting AS drs  WHERE d.datname = 'test' AND d.oid = drs.setdatabase;
 setdatabase | setrole |      setconfig      
-------------+---------+---------------------
       16384 |       0 | {search_path=test}
       16384 |   16385 | {search_path=other}
(2 rows)

This will probably work anyway as readSearchPath will take the first which is most probably the good one but I think the ordering isn't guarantee

searchPathPrefix := "search_path"
for _, v := range roleConfig {
config := string(v)
if strings.HasPrefix(config, searchPathPrefix) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I discovered pg_options_to_table thanks to you, we should probably use it here too.

If you don't have time I can do it in another PR though

@cyrilgdn
Copy link
Owner

@NitriKx Thanks for your work on that 🙏

@cyrilgdn cyrilgdn added the waiting-response Further information is requested label Mar 2, 2024
@NitriKx
Copy link
Author

NitriKx commented Mar 8, 2024

Thanks for your feedback @cyrilgdn, will try to do the changes (and rebase) next week 🙂

@github-actions github-actions bot removed the waiting-response Further information is requested label Mar 8, 2024
@cyrilgdn cyrilgdn force-pushed the main branch 3 times, most recently from f2c2e47 to dea1401 Compare September 8, 2024 17:06
@teru01
Copy link

teru01 commented Sep 18, 2024

@NitriKx
Hi. Do you have any progress on it?
I can inherit this PR if you don't have enough time because I need the search_path feature as well.

@zam6ak
Copy link

zam6ak commented Feb 1, 2025

@NitriKx CC @cyrilgdn
Being able to set search_path on DB level is really useful.
Any chance to get this merged?

@NitriKx
Copy link
Author

NitriKx commented Feb 1, 2025

Heey will try to work on this next week sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants